linux-kernel - Re: [PATCH] checkpatch: prefer = {} initializations to = {0}

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]

Message-ID: <20210814144427.GZ22278@shell.armlinux.org.uk>
Date:   Sat, 14 Aug 2021 15:44:27 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        Leon Romanovsky <leon@...nel.org>,
        Joe Perches <joe@...ches.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Andy Whitcroft <apw@...onical.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        Julia Lawall <julia.lawall@...ia.fr>
Subject: Re: [PATCH] checkpatch: prefer = {} initializations to = {0}

On Sat, Aug 14, 2021 at 03:59:22PM +0200, Christophe JAILLET wrote:
> Hi all,
> 
> Le 05/08/2021 à 12:43, Dan Carpenter a écrit :
> > The "= {};" style empty struct initializer is preferred over = {0}.
> > It avoids the situation where the first struct member is a pointer and
> > that generates a Sparse warning about assigning using zero instead of
> > NULL.  Also it's just nicer to look at.
> > 
> > Some people complain that {} is less portable but the kernel has
> > different portability requirements from userspace so this is not a
> > issue that we care about.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> > ---
> >   scripts/checkpatch.pl | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 461d4221e4a4..32c8a0ca6fd0 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -4029,6 +4029,12 @@ sub process {
> >   			     "Using $1 is unnecessary\n" . $herecurr);
> >   		}
> > +# prefer = {}; to = {0};
> > +		if ($line =~ /= \{ *0 *\}/) {
> > +			WARN("ZERO_INITIALIZER",
> > +			     "= {} is preferred over = {0}\n" . $herecurr);
> > +		}
> > +
> >   # Check for potential 'bare' types
> >   		my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
> >   		    $realline_next);
> > 
> 
> [1] and [2] state that {} and {0} don't have the same effect. So if correct,
> this is not only a matter of style.

This comes down to the fundamental question whether initialising a
pointer with the integer 0 is permitted or not. The first thing to
realise is that C allows integer 0 to be implicitly the null pointer.

So, initialising a pointer to integer 0 sets it to the null pointer.
Hence:
void *bar = 0;

is entirely legal, as is.
struct foo {
	void *bar;
};
struct foo foo = {0};

Things get more complex when you have:
struct foo {
	int bar[16];
};
struct foo foo = {0};

or:

struct foo {
	struct bar bar;
};
struct foo foo = {0};

Depending on your GCC version, GCC may or may not emit:

warning: missing braces around initializer [-Wmissing-braces]

Clang does emit a very similar warning (see the thread that Vladimir
linked to in the links in your email.)

C99 does permit this, but in terms of portability across the currently
supported compilers for the kernel, some will warn as per the above.
So, it's best to ensure that the extra { } are present, or just avoid
using the {0} notation and use the empty initialiser { }.

> When testing with gcc 10.3.0, I arrived at the conclusion that both {} and
> {0} HAVE the same behavior (i.e the whole structure and included structures
> are completely zeroed) and I don't have a C standard to check what the rules
> are.

It should do. C defines that unmentioned members in an initialiser
shall be "initialised implicitly the same as objects that have static
storage duration". Given the platforms we currently support with the
kernel, that basically means unmentioned members will be zeroed. The
empty initialiser lists no members, so all members are not mentioned,
so this applies.

Lastly, {0} is extra unnecessary typing. Apart from one's style (which
I think having warning-free builds trumps), I'm not sure why you'd
want the extra work of typing the extra 0. :)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!