Messages in this thread | | | Date | Fri, 1 Sep 2023 09:59:20 +0100 | Subject | Re: [PATCH 1/2] dma/pool: trivial: add semicolon after label attributes | From | Robin Murphy <> |
| |
On 2023-09-01 09:56, Robin Murphy wrote: > On 2023-08-31 12:59, Chunhui He wrote: >> >> 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. > > And that commit was also wrong. Nobody suggested C11 doesn't support > inline declarations - it demonstrably does - the fact in question is > that *attributes* on declarations is a C++ thing and not valid in C:
Argh, sorry, s/attributes/labels/
/me goes to make more coffee...
Robin.
> ~/src/linux$ git diff > diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c > index 1acec2e22827..e1354235cb9c 100644 > --- a/kernel/dma/pool.c > +++ b/kernel/dma/pool.c > @@ -137,7 +137,8 @@ static int atomic_pool_expand(struct gen_pool *pool, > size_t pool_size, > dma_common_free_remap(addr, pool_size); > #endif > free_page: __maybe_unused > - __free_pages(page, order); > + int x = order; > + __free_pages(page, x); > out: > return ret; > } > ~/src/linux$ make -j32 > CALL scripts/checksyscalls.sh > CC kernel/dma/pool.o > kernel/dma/pool.c: In function ‘atomic_pool_expand’: > kernel/dma/pool.c:140:2: error: a label can only be part of a statement > and a declaration is not a statement > 140 | int x = order; > | ^~~ > make[4]: *** [scripts/Makefile.build:243: kernel/dma/pool.o] Error 1 > make[3]: *** [scripts/Makefile.build:480: kernel/dma] Error 2 > make[2]: *** [scripts/Makefile.build:480: kernel] Error 2 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [/home/robmur01/src/linux/Makefile:2032: .] Error 2 > make: *** [Makefile:234: __sub-make] Error 2 > > > Thanks, > Robin. >
| |