linux-kernel - Re: [PATCH] kgdb: fix gcc-11 warning on indentation

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 22 Mar 2021 10:04:57 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Jason Wessel <jason.wessel@...driver.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ingo Molnar <mingo@...e.hu>,
        Christian Brauner <christian.brauner@...ntu.com>,
        kgdb-bugreport@...ts.sourceforge.net,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kgdb: fix gcc-11 warning on indentation

Hi,

On Mon, Mar 22, 2021 at 9:43 AM Arnd Bergmann <arnd@...nel.org> wrote:
>
> -#define v1printk(a...) do { \
> -       if (verbose) \
> -               printk(KERN_INFO a); \
> -       } while (0)
> -#define v2printk(a...) do { \
> -       if (verbose > 1) \
> -               printk(KERN_INFO a); \
> -               touch_nmi_watchdog();   \
> -       } while (0)
> -#define eprintk(a...) do { \
> -               printk(KERN_ERR a); \
> -               WARN_ON(1); \
> -       } while (0)
> +#define v1printk(a...) do {            \

nit: In addition to the indentation change you're also lining up the
backslashes. Is that just personal preference, or is there some
official recommendation in the kernel? I don't really have a strong
opinion either way (IMO each style has its advantages).


> +       if (verbose)                    \
> +               printk(KERN_INFO a);    \
> +} while (0)
> +#define v2printk(a...) do {            \
> +       if (verbose > 1)                \
> +               printk(KERN_INFO a);    \
> +       touch_nmi_watchdog();           \

This touch_nmi_watchdog() is pretty wonky. I guess maybe the
assumption is that the "verbose level 2" prints are so chatty that the
printing might prevent us from touching the NMI watchdog in the way
that we normally do and thus we need an extra one here?

...but, in that case, I think the old code was _wrong_ and that the
intention was that the touch_nmi_watchdog() should only be if "verose
> 1" as the indentation implied. There doesn't feel like a reason to
touch the watchdog if we're not doing anything slow.

-Doug