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

From: Robin Murphy
Date: Tue Aug 29 2023 - 11:28:57 EST


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.

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.