# [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback 5 messages from 2021-06-01 to 2021-06-01. Participants: Ævar Arnfjörð Bjarmason, Johannes Sixt, René Scharfe, Junio C Hamano. Thread: https://gitlist.dev/t/55812 ## Ævar Arnfjörð Bjarmason, 2021-06-01 00:05 Subject: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback Message-ID: URL: https://gitlist.dev/e/patch-1.1-f109a61b11-20210601T000433Z-avarab%40gmail.com ``` Fix a warning on AIX's xlc compiler that's been emitted since my a1aad71601a (fsck.h: use "enum object_type" instead of "int", 2021-03-28): "builtin/fsck.c", line 805.32: 1506-068 (W) Operation between types "int(*)(struct object*,enum object_type,void*,struct fsck_options*)" and "int(*)(struct object*,int,void*,struct fsck_options*)" is not allowed. I.e. it complains about us assigning a function with a prototype "int" where we're expecting "enum object_type". Enums are just ints in C, but it seems xlc is more picky than some about conflating them at the source level. Signed-off-by: Ævar Arnfjörð Bjarmason --- This is new in v2.32.0, so sending this during the rc phase, just a warning though, so unless you're using fatal warnings on that OS/platform it won't impact anything, and even then it's just a minor annoyance... builtin/fsck.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 87a99b0108..b42b6fe21f 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -109,7 +109,8 @@ static int fsck_error_func(struct fsck_options *o, static struct object_array pending; -static int mark_object(struct object *obj, int type, void *data, struct fsck_options *options) +static int mark_object(struct object *obj, enum object_type type, + void *data, struct fsck_options *options) { struct object *parent = data; -- 2.32.0.rc1.460.g26a014da44c ``` ## Johannes Sixt, 2021-06-01 06:25 Subject: Re: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback Message-ID: URL: https://gitlist.dev/e/de9dea3a-6138-5e2f-7dc8-8a2ddf28bb84%40kdbg.org In-Reply-To: ``` Am 01.06.21 um 02:05 schrieb Ævar Arnfjörð Bjarmason: > Fix a warning on AIX's xlc compiler that's been emitted since my > a1aad71601a (fsck.h: use "enum object_type" instead of "int", > 2021-03-28): > > "builtin/fsck.c", line 805.32: 1506-068 (W) Operation between > types "int(*)(struct object*,enum object_type,void*,struct > fsck_options*)" and "int(*)(struct object*,int,void*,struct > fsck_options*)" is not allowed. > > I.e. it complains about us assigning a function with a prototype "int" > where we're expecting "enum object_type". Enums are just ints in C, > but it seems xlc is more picky than some about conflating them at the > source level. Is that true? I thought compilers were allowed to use whatever data type is sufficient to represent all enumeration values. For this reason, you sometimes see enum X { A, B, X_MAX = 0x7fffffff }; that ensures that an int must be used for representation of enum X. So, AFAICS, your patch is an actual fix, not just cosmetic. > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > > This is new in v2.32.0, so sending this during the rc phase, just a > warning though, so unless you're using fatal warnings on that > OS/platform it won't impact anything, and even then it's just a minor > annoyance... > > builtin/fsck.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/builtin/fsck.c b/builtin/fsck.c > index 87a99b0108..b42b6fe21f 100644 > --- a/builtin/fsck.c > +++ b/builtin/fsck.c > @@ -109,7 +109,8 @@ static int fsck_error_func(struct fsck_options *o, > > static struct object_array pending; > > -static int mark_object(struct object *obj, int type, void *data, struct fsck_options *options) > +static int mark_object(struct object *obj, enum object_type type, > + void *data, struct fsck_options *options) > { > struct object *parent = data; > > ``` ## Ævar Arnfjörð Bjarmason, 2021-06-01 08:04 Subject: Re: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback Message-ID: <87k0ne0zx3.fsf@evledraar.gmail.com> URL: https://gitlist.dev/e/87k0ne0zx3.fsf%40evledraar.gmail.com In-Reply-To: ``` On Tue, Jun 01 2021, Johannes Sixt wrote: > Am 01.06.21 um 02:05 schrieb Ævar Arnfjörð Bjarmason: >> Fix a warning on AIX's xlc compiler that's been emitted since my >> a1aad71601a (fsck.h: use "enum object_type" instead of "int", >> 2021-03-28): >> >> "builtin/fsck.c", line 805.32: 1506-068 (W) Operation between >> types "int(*)(struct object*,enum object_type,void*,struct >> fsck_options*)" and "int(*)(struct object*,int,void*,struct >> fsck_options*)" is not allowed. >> >> I.e. it complains about us assigning a function with a prototype "int" >> where we're expecting "enum object_type". Enums are just ints in C, >> but it seems xlc is more picky than some about conflating them at the >> source level. > > Is that true? I thought compilers were allowed to use whatever data type > is sufficient to represent all enumeration values. For this reason, you > sometimes see > > enum X { A, B, X_MAX = 0x7fffffff }; > > that ensures that an int must be used for representation of enum X. So, > AFAICS, your patch is an actual fix, not just cosmetic. The identifiers in an enumerator list are declared as constants that have type int and may appear wherever such are permitted. [...] Each enumerated type shall be compatible with char,asigned integer type, or an unsigned integer type. The choice of type is implementation-defined,110) but shall be capable of representing the values of all the members of the enumeration [...] Thus, the identifiers of enumeration constants declared in the same scope shall all be distinct from each other and from other identifiers declared in ordinary declarators --C99, 6.7.2.2 @ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf My reading of that is that mixing the two (which we indeed, do all over the place) is guaranteed to work, we've got plenty of places where e.g. enum object_type is passed to something else as an "int". This xlc warning in particular probably has nothing per-se to do with enum v.s. int, but just that it's complaining that a function pointer doesn't have exactly the expected type. >> >> Signed-off-by: Ævar Arnfjörð Bjarmason >> --- >> >> This is new in v2.32.0, so sending this during the rc phase, just a >> warning though, so unless you're using fatal warnings on that >> OS/platform it won't impact anything, and even then it's just a minor >> annoyance... >> >> builtin/fsck.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/builtin/fsck.c b/builtin/fsck.c >> index 87a99b0108..b42b6fe21f 100644 >> --- a/builtin/fsck.c >> +++ b/builtin/fsck.c >> @@ -109,7 +109,8 @@ static int fsck_error_func(struct fsck_options *o, >> >> static struct object_array pending; >> >> -static int mark_object(struct object *obj, int type, void *data, struct fsck_options *options) >> +static int mark_object(struct object *obj, enum object_type type, >> + void *data, struct fsck_options *options) >> { >> struct object *parent = data; >> >> ``` ## René Scharfe, 2021-06-01 17:56 Subject: Re: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback Message-ID: <8df5f832-adaa-7cb0-38e3-1a4b2bcff252@web.de> URL: https://gitlist.dev/e/8df5f832-adaa-7cb0-38e3-1a4b2bcff252%40web.de In-Reply-To: <87k0ne0zx3.fsf@evledraar.gmail.com> ``` Am 01.06.21 um 10:04 schrieb Ævar Arnfjörð Bjarmason: > > On Tue, Jun 01 2021, Johannes Sixt wrote: > >> Am 01.06.21 um 02:05 schrieb Ævar Arnfjörð Bjarmason: >>> Fix a warning on AIX's xlc compiler that's been emitted since my >>> a1aad71601a (fsck.h: use "enum object_type" instead of "int", >>> 2021-03-28): >>> >>> "builtin/fsck.c", line 805.32: 1506-068 (W) Operation between >>> types "int(*)(struct object*,enum object_type,void*,struct >>> fsck_options*)" and "int(*)(struct object*,int,void*,struct >>> fsck_options*)" is not allowed. >>> >>> I.e. it complains about us assigning a function with a prototype "int" >>> where we're expecting "enum object_type". Enums are just ints in C, >>> but it seems xlc is more picky than some about conflating them at the >>> source level. >> >> Is that true? I thought compilers were allowed to use whatever data type >> is sufficient to represent all enumeration values. For this reason, you >> sometimes see >> >> enum X { A, B, X_MAX = 0x7fffffff }; >> >> that ensures that an int must be used for representation of enum X. So, >> AFAICS, your patch is an actual fix, not just cosmetic. > > The identifiers in an enumerator list are declared as constants > that have type int and may appear wherever such are > permitted. [...] Each enumerated type shall be compatible with > char,asigned integer type, or an unsigned integer type. The > choice of type is implementation-defined,110) but shall be > capable of representing the values of all the members of the > enumeration [...] Thus, the identifiers of enumeration constants > declared in the same scope shall all be distinct from each other > and from other identifiers declared in ordinary declarators > > --C99, 6.7.2.2 @ > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf > > My reading of that is that mixing the two (which we indeed, do all over > the place) is guaranteed to work, we've got plenty of places where > e.g. enum object_type is passed to something else as an "int". > > This xlc warning in particular probably has nothing per-se to do with > enum v.s. int, but just that it's complaining that a function pointer > doesn't have exactly the expected type. The object_type item OBJ_BAD has the value -1. If it had a low positive value instead, GCC and Clang would warn. Demo: https://godbolt.org/z/vKPdjrYsa As you cited above, "The choice of type is implementation-defined". You can use enum values as if they were integers, but that doesn't dictate their storage size. I find the lack of a warning depending on the value range disturbing. Perhaps it's omitted because GCC and Clang guarantee compatibility of such an enum and int for all prior versions (i.e. the implementation- defined specifics never changed). A stricter check like xlc does is more useful for portability, though. >>> >>> Signed-off-by: Ævar Arnfjörð Bjarmason >>> --- >>> >>> This is new in v2.32.0, so sending this during the rc phase, just a >>> warning though, so unless you're using fatal warnings on that >>> OS/platform it won't impact anything, and even then it's just a minor >>> annoyance... >>> >>> builtin/fsck.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/builtin/fsck.c b/builtin/fsck.c >>> index 87a99b0108..b42b6fe21f 100644 >>> --- a/builtin/fsck.c >>> +++ b/builtin/fsck.c >>> @@ -109,7 +109,8 @@ static int fsck_error_func(struct fsck_options *o, >>> >>> static struct object_array pending; >>> >>> -static int mark_object(struct object *obj, int type, void *data, struct fsck_options *options) >>> +static int mark_object(struct object *obj, enum object_type type, >>> + void *data, struct fsck_options *options) >>> { >>> struct object *parent = data; >>> >>> > ``` ## Junio C Hamano, 2021-06-01 21:04 Subject: Re: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback Message-ID: URL: https://gitlist.dev/e/xmqq8s3ts3ds.fsf%40gitster.g In-Reply-To: ``` Ævar Arnfjörð Bjarmason writes: > Fix a warning on AIX's xlc compiler that's been emitted since my > a1aad71601a (fsck.h: use "enum object_type" instead of "int", > 2021-03-28): > > "builtin/fsck.c", line 805.32: 1506-068 (W) Operation between > types "int(*)(struct object*,enum object_type,void*,struct > fsck_options*)" and "int(*)(struct object*,int,void*,struct > fsck_options*)" is not allowed. > > I.e. it complains about us assigning a function with a prototype "int" > where we're expecting "enum object_type". Enums are just ints in C, > but it seems xlc is more picky than some about conflating them at the > source level. As others already have pointed out, xlc is correct and these warnings are perhaps useful. I'd just remove the last sentence while queuing, as "Enums are just ints in C" is only half a truth. The constants like OBJ_COMMIT can be used as "int", but the type of an enum itself may not be the same as "int"---it can be narrower. > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > > This is new in v2.32.0, so sending this during the rc phase, just a > warning though, so unless you're using fatal warnings on that > OS/platform it won't impact anything, and even then it's just a minor > annoyance... Thanks. > > builtin/fsck.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/builtin/fsck.c b/builtin/fsck.c > index 87a99b0108..b42b6fe21f 100644 > --- a/builtin/fsck.c > +++ b/builtin/fsck.c > @@ -109,7 +109,8 @@ static int fsck_error_func(struct fsck_options *o, > > static struct object_array pending; > > -static int mark_object(struct object *obj, int type, void *data, struct fsck_options *options) > +static int mark_object(struct object *obj, enum object_type type, > + void *data, struct fsck_options *options) > { > struct object *parent = data; ```