Re: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback

["René Scharfe" <l.s.r@web.de>]

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 <avarab@gmail.com>
>>> ---
>>>
>>> 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;
>>>
>>>
>