Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 31 Aug 2023 11:59:37 +0000 (UTC) | Subject | Re: [PATCH 1/2] dma/pool: trivial: add semicolon after label attributes | From | Chunhui He <> |
| |
On Tue, 29 Aug 2023 16:28:05 +0100, Robin Murphy <robin.murphy@arm.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.
| ![\](/images/icornerr.gif) |