Re: [PATCH 1/2] dma/pool: trivial: add semicolon after label attributes

[Chunhui He]

On Tue, 29 Aug 2023 16:28:05 +0100, Robin Murphy <robin.murphy@xxxxxxx> wrote:
> On 29/08/2023 4:12 pm, Christoph Hellwig wrote:
>> On Tue, Aug 29, 2023 at 03:22:22PM +0100, Robin Murphy wrote:
>>> AFAICS, what that clearly says is that *C++* label attributes can be
>>> ambiguous. This is not C++ code. Even in C11, declarations still
>>> cannot be
>>> labelled, so it should still be the case that, per the same GCC
>>> documentation, "the ambiguity does not arise". And even if the
>>> language did
>>> allow it, an inline declaration at that point at the end of a function
>>> would be downright weird and against the kernel coding style anyway.
>>>
>>> So, I don't really see what's "better" about cluttering up C code with
>>> unnecessary C++isms; it's just weird noise to me. The only thing I
>>> think it
>>> *does* achieve is introduce the chance that the static checker brigade
>>> eventually identifies a redundant semicolon and we get more patches to
>>> remove it again.

Inline declaration is a GNU C extension, so the ambiguity may arise.
Adding ';' makes the compiler easier to parse correctly, so I say
"better". The commit 13a453c241b78934a945b1af572d0533612c9bd1
(sched/fair: Add ';' after label attributes) also says the same.

>> Agreed.  Even more importantly that attribute looks rather
>> questionable
>> to start with as it can be dropped by just moving the #endif a little:
>> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
>> index 1acec2e228273f..da03c4a57cebe3 100644
>> --- a/kernel/dma/pool.c
>> +++ b/kernel/dma/pool.c
>> @@ -135,8 +135,8 @@ static int atomic_pool_expand(struct gen_pool
>> *pool, size_t pool_size,
>>   remove_mapping:
>>   #ifdef CONFIG_DMA_DIRECT_REMAP
>>   	dma_common_free_remap(addr, pool_size);
>> +free_page:
>>   #endif
>> -free_page: __maybe_unused
>>   	__free_pages(page, order);
>>   out:
>>   	return ret;
> 
> Oh, indeed - I hadn't really looked at the context itself. My
> non-exhaustive grep skills show a couple of hundred instances of
> label-above-#endif vs. three (!) instances of __maybe_unused, so ack
> to making that cleanup to just remove the question entirely.
> 
> Cheers,
> Robin.

I agree label-above-#endif remove the question entirely.

Cheers,
Chunhui.