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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230831.115937.924195103727242070.hchunhui@mail.ustc.edu.cn>
Date:   Thu, 31 Aug 2023 11:59:37 +0000 (UTC)
From:   Chunhui He <hchunhui@...l.ustc.edu.cn>
To:     robin.murphy@....com
Cc:     hch@....de, m.szyprowski@...sung.com, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dma/pool: trivial: add semicolon after label
 attributes


On Tue, 29 Aug 2023 16:28:05 +0100, Robin Murphy <robin.murphy@....com> 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.