Re: [PATCH] builtin/fsck.c: don't conflate "int" and "enum" in callback
Am 01.06.21 um 10:04 schrieb Ævar Arnfjörð Bjarmason:
Show 47 quoted lines
>
> 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.
Show 28 quoted lines
>>>
>>> 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;
>>>
>>>
>
Ævar Arnfjörð Bjarmason
