[PATCH 6/6] checkpatch.pl: Request if() instead #ifdef

[Simon Glass]

Hi Akashi,

On Sun, 14 Jun 2020 at 20:59, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> On Thu, Jun 04, 2020 at 07:39:35PM -0400, Tom Rini wrote:
> > On Fri, May 22, 2020 at 04:32:40PM -0600, Simon Glass wrote:
> >
> > > There is a lot of use of #ifdefs in U-Boot. In an effort reduce this,
> > > suggest using the compile-time construct.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> >
> > Applied to u-boot/master, thanks!
>
> This check is simple, but IMHO, too simple.
> It will generate false-positive, or pointless, warnings
> for some common use cases. Say,
>
> In an include header,
> #ifdef CONFIG_xxx
> extern int foo_data;
> int foo(void);
> #endif

We should try to avoid this in header files. But I sent a patch
earlier today to turn off the check for header files and device tree.

>
> Or in a C file (foo_common.c),
> #ifdef CONFIG_xxx_a
> int foo_a(void)
> ...
> #endif
> #ifdef CONFIG_xxx_b
> int foo_b(void)
> ...
> #endif
>

Perhaps the if() could be inside those functions in some cases?

> Or,
>
> struct baa baa_list[] = {
> #ifdef CONFIG_xxx
>         data_xxx,
> #endif

I'm not sure how to handle this one.

> ...
>
> They are harmless and can be ignored, but also annoying.
> Can you sophisticate this check?

Yes I agree we should avoid false negatives. It is better not to have
a check than have one that is unreliable.

>
> In addition, if I want to stick to this rule, there can co-exist
> an "old" style and "new" style of code in a single file.
> (particularly tons of examples in UEFI subsystem)
>
> How should we deal with this?

Convert it?

>
> Thanks,
> -Takahiro Akashi

Regards,
Simon