Hi,
> On 9 Jun 2023, at 15:19, Julien Grall <jul...@xen.org> wrote:
>
> Hi Jan,
>
> On 09/06/2023 09:54, Jan Beulich wrote:
>> On 08.06.2023 14:18, Roberto Bagnara wrote:
>>> On 07/06/23 09:39, Jan Beulich wrote:
>>>> On 05.06.2023 15:26, Roberto Bagnara wrote:
>>>>> On 05/06/23 11:28, Jan Beulich wrote:
>>>>>> On 05.06.2023 07:28, Roberto Bagnara wrote:
>>>>> You are right: here are a few examples for U2:
>>>>>
>>>>> xen/arch/arm/cpuerrata.c:92.12-92.35:
>>>>> empty initializer list (ill-formed for the C99 standard, ISO/IEC
>>>>> 9899:1999 Section 6.7.8: "An empty initialization list." [STD.emptinit]).
>>>>> Tool used is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>> xen/include/xen/spinlock.h:31.21-31.23: expanded from macro `_LOCK_DEBUG'
>>>>> xen/include/xen/spinlock.h:143.57-143.67: expanded from macro
>>>>> `SPIN_LOCK_UNLOCKED'
>>>>> xen/include/xen/spinlock.h:144.43-144.60: expanded from macro
>>>>> `DEFINE_SPINLOCK'
>>>>
>>>> I'm afraid this is a bad example, as it goes hand-in-hand with using
>>>> another extension. I don't think using a non-empty initialization list
>>>> is going to work with
>>>>
>>>> union lock_debug { };
>>>
>>> Yes, this is C99 undefined behavior 58:
>>> "A structure or union is defined as containing no named members (6.7.2.1)."
>>>
>>> Here is another example:
>>>
>>> lpae_t pte = {};
>>>
>>> whereas we have
>>>
>>> typedef union {
>>> uint64_t bits;
>>> lpae_pt_t pt;
>>> lpae_p2m_t p2m;
>>> lpae_walk_t walk;
>>> } lpae_t;
>>>
>>>
>>>>> xen/arch/arm/cpuerrata.c:678.5-678.6:
>>>>> empty initializer list (ill-formed for the C99 standard, ISO/IEC
>>>>> 9899:1999 Section 6.7.8: "An empty initialization list." [STD.emptinit]).
>>>>> Tool used is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>>
>>>>> xen/arch/arm/cpufeature.c:33.5-33.6:
>>>>> empty initializer list (ill-formed for the C99 standard, ISO/IEC
>>>>> 9899:1999 Section 6.7.8: "An empty initialization list." [STD.emptinit]).
>>>>> Tool used is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>
>>>> Both of these are a common idiom we use: The "sentinel" of an array
>>>> of compound type initializer.
>>>
>>> Wouldn't it be possible writing such sentinels in a standard-compliant
>>> way, like {0} or similar, instead of {}?
>> I would be possible, sure, but the question is whether we want that. Iirc
>> in review comments we've been asking to preferably use {}, for being
>> shorter / less clutter without resulting in any ambiguity.
>>>>>>> U6) Empty declarations.
>>>>>
>>>>> Examples:
>>>>>
>>>>> xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
>>>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999
>>>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is
>>>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>>
>>>>> xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
>>>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999
>>>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is
>>>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>
>>>> Looks like these could be taken care of by finally purging our
>>>> EXPORT_SYMBOL() stub.
>>>>
>>>>> xen/arch/arm/include/asm/vreg.h:143.26:
>>>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999
>>>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is
>>>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>>
>>>>> xen/arch/arm/include/asm/vreg.h:144.26:
>>>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999
>>>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is
>>>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>>
>>>> I'm having trouble spotting anything suspicious there.
>>>
>>> The macro expands to definitions of inline functions
>>> and after the macro invocation there is a ";".
>>>
>>> The preprocessed code is then:
>>>
>>> static inline void foo() { ... }
>>> ;
>>>
>>> where the final ";" is an empty declaration not allowed by
>>> the C99 language standard.
>> Oh, I see.
>>> Removing the ";" after the macro invocation is a possible solution,
>>> but other possibilities exist if this is strongly unwanted.
>> We have other macros to instantiate functions, and there no stray
>> semicolons are used. I think this wants doing the same way here, but it
>> being Arm code the ultimate say is with the Arm maintainers.
>
> I don't think there is a reason to keep the ";" after. So I would be fine if
> this is removed.
+1
Cheers
Bertrand
