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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 1, 2021, 08:04 UTC
Message-ID: <87k0ne0zx3.fsf@evledraar.gmail.com>
In-Reply-To: <de9dea3a-6138-5e2f-7dc8-8a2ddf28bb84@kdbg.org>

On Tue, Jun 01 2021, Johannes Sixt wrote:

Show 23 quoted lines

> 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.

Show 27 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;
>>  
>>