[PATCH] checkpatch: prefer = {} initializations to = {0}

[PATCH] checkpatch: prefer = {} initializations to = {0}

[Dan Carpenter]

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@oracle.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);
-- 
2.30.2

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

[Joe Perches]

On Thu, 2021-08-05 at 13:43 +0300, Dan Carpenter wrote:
> 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.
[]
> diff --git 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 *\}/) {

Have you done a grep and looked at this pattern's actual use in the kernel?
I rather doubt it.

> +			WARN("ZERO_INITIALIZER",
> +			     "= {} is preferred over = {0}\n" . $herecurr);
> +		}
> +

To be much more specific to the actual desired match as stated in
the possible commit log, this should probably be something like:

	if (defined($stat) &&
	    $stat =~ /^\+\s+$Declare\s*$Ident\s*=\s*\{\s*0\s*\}\s*;/) {

and maybe it should be a --strict CHK and not WARN.

And I generally avoid using "we" in a commit message.

Maybe something like:
---
 scripts/checkpatch.pl | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 461d4221e4a4a..e3d4aa5f86c46 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6911,6 +6911,21 @@ sub process {
 			     "externs should be avoided in .c files\n" .  $herecurr);
 		}
 
+# check for non-global single initializers with '= {0};', prefer '= {};'
+# there is already a GLOBAL_INITIALIZERS test so avoid those too with /^\+\s+/
+		if (defined($stat) &&
+		    $stat =~ /^\+\s+$Declare\s*$Ident\s*(=\s*\{\s*0\s*\}\s*);/) {
+			my $match = $1;
+			my $cnt = statement_rawlines($stat);
+			my $herectx = get_stat_here($linenr, $cnt, $here);
+			if (WARN("ZERO_INITIALIZER",
+				 "Prefer '= {}' over '$match'\n" . $herectx) &&
+			    $fix &&
+			    $cnt == 1) {	# can only --fix single line statements
+				$fixed[$fixlinenr] =~ s/\s*=\s*\{\s*0\s*\}\s*;/ = {};/;
+			}
+		}
+
 # check for function declarations that have arguments without identifier names
 		if (defined $stat &&
 		    $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s &&

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

[Joe Perches]

On Thu, 2021-08-05 at 05:27 -0700, Joe Perches wrote:
> On Thu, 2021-08-05 at 13:43 +0300, Dan Carpenter wrote:
> > 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.

Perhaps a cocci script like the below could help too:

$ cat zero_init_struct.cocci
@@
identifier name;
identifier t;
@@

	struct name t = {
-	       0
	};

@@
identifier name;
identifier t;
identifier member;
@@

	struct name t = {
	       ...,
		.member = {
-		0
		},
		...,
	};

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

[Julia Lawall]

On Thu, 5 Aug 2021, Joe Perches wrote:

> On Thu, 2021-08-05 at 05:27 -0700, Joe Perches wrote:
> > On Thu, 2021-08-05 at 13:43 +0300, Dan Carpenter wrote:
> > > 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.
>
> Perhaps a cocci script like the below could help too:
>
> $ cat zero_init_struct.cocci
> @@
> identifier name;
> identifier t;
> @@
>
> 	struct name t = {
> -	       0
> 	};
>
> @@
> identifier name;
> identifier t;
> identifier member;
> @@
>
> 	struct name t = {
> 	       ...,
> 		.member = {
> -		0
> 		},
> 		...,
> 	};

My test turns up over 1900 occurrences.  There is the question of whether
{} or { } is preferred.  The above semantic patch replaces {0} by {} and
( 0 } by { }.

julia

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

[Joe Perches]

On Thu, 2021-08-05 at 20:17 +0200, Julia Lawall wrote:
> On Thu, 5 Aug 2021, Joe Perches wrote:
> > On Thu, 2021-08-05 at 05:27 -0700, Joe Perches wrote:
> > > On Thu, 2021-08-05 at 13:43 +0300, Dan Carpenter wrote:
> > > > 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.
> > 
> > Perhaps a cocci script like the below could help too:
> > 
> > $ cat zero_init_struct.cocci
> > @@
> > identifier name;
> > identifier t;
> > @@
> > 
> > 	struct name t = {
> > -	       0
> > 	};
> > 
> > @@
> > identifier name;
> > identifier t;
> > identifier member;
> > @@
> > 
> > 	struct name t = {
> > 	       ...,
> > 		.member = {
> > -		0
> > 		},
> > 		...,
> > 	};
> 
> My test turns up over 1900 occurrences.  There is the question of whether
> {} or { } is preferred.  The above semantic patch replaces {0} by {} and
> ( 0 } by { }.

I saw that and I don't recall how to force one style or another
to be output.

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

[Julia Lawall]

On Thu, 5 Aug 2021, Joe Perches wrote:

> On Thu, 2021-08-05 at 20:17 +0200, Julia Lawall wrote:
> > On Thu, 5 Aug 2021, Joe Perches wrote:
> > > On Thu, 2021-08-05 at 05:27 -0700, Joe Perches wrote:
> > > > On Thu, 2021-08-05 at 13:43 +0300, Dan Carpenter wrote:
> > > > > 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.
> > >
> > > Perhaps a cocci script like the below could help too:
> > >
> > > $ cat zero_init_struct.cocci
> > > @@
> > > identifier name;
> > > identifier t;
> > > @@
> > >
> > > 	struct name t = {
> > > -	       0
> > > 	};
> > >
> > > @@
> > > identifier name;
> > > identifier t;
> > > identifier member;
> > > @@
> > >
> > > 	struct name t = {
> > > 	       ...,
> > > 		.member = {
> > > -		0
> > > 		},
> > > 		...,
> > > 	};
> >
> > My test turns up over 1900 occurrences.  There is the question of whether
> > {} or { } is preferred.  The above semantic patch replaces {0} by {} and
> > ( 0 } by { }.
>
> I saw that and I don't recall how to force one style or another
> to be output.

If you remove something and put it back, then Coccinelle takes care of
pretty printing it.  So the following produces {} everywhere.  Fortunately
Dan seems to prefer that...

@@
identifier name;
identifier t;
@@

        struct name t =
-              {0}
+              {}
        ;

@@
identifier name;
identifier t;
identifier member;
@@

        struct name t = {
               ...,
                .member =
-               {0}
+               {}
                ,
                ...,
        };

julia

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

[Christophe JAILLET]

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@oracle.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.

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.
gcc online doc didn't help me either.

To test, I wrote a trivial C program, compiled it with gcc -S and looked 
at the assembly files.


Maybe, if it is an undefined behavior, other compilers behave 
differently than gcc.


However, the 2 persons listed bellow have a much better Linux and C 
background than me. So it is likely that my testings were too naive.


Can someone provide some rational or compiler output that confirms that 
{} and {0} are not the same?

Because if confirmed, I guess that there is some clean-up work to do all 
over the code, not only to please Sparse!


Thanks in advance.
CJ



[1]: Russell King - 
https://lore.kernel.org/netdev/YRFGxxkNyJDxoGWu@shredder/T/#efe1b6c7862b7ca9588c2734f04be5ef94e03d446

[2]: Leon Romanovsky - 
https://lore.kernel.org/netdev/YRFGxxkNyJDxoGWu@shredder/T/#efe1b6c7862b7ca9588c2734f04be5ef94e03d446

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

[Marion & Christophe JAILLET]

Copy paste error, see below :/

Le 14/08/2021 à 15:59, Christophe JAILLET a écrit :
> 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@oracle.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.
> 
> 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.
> gcc online doc didn't help me either.
> 
> To test, I wrote a trivial C program, compiled it with gcc -S and looked 
> at the assembly files.
> 
> 
> Maybe, if it is an undefined behavior, other compilers behave 
> differently than gcc.
> 
> 
> However, the 2 persons listed bellow have a much better Linux and C 
> background than me. So it is likely that my testings were too naive.
> 
> 
> Can someone provide some rational or compiler output that confirms that 
> {} and {0} are not the same?
> 
> Because if confirmed, I guess that there is some clean-up work to do all 
> over the code, not only to please Sparse!
> 
> 
> Thanks in advance.
> CJ
> 
> 
> 
> [1]: Russell King - 
> https://lore.kernel.org/netdev/YRFGxxkNyJDxoGWu@shredder/T/#efe1b6c7862b7ca9588c2734f04be5ef94e03d446 
> 
> 
> [2]: Leon Romanovsky - 
> https://lore.kernel.org/netdev/YRFGxxkNyJDxoGWu@shredder/T/#efe1b6c7862b7ca9588c2734f04be5ef94e03d446 
https://lore.kernel.org/netdev/162894660670.3097.4150652110351873021.git-patchwork-notify@kernel.org/T/#m3424d6e97ef0f0ddd429cce3369a6da0ea9af276

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

[Leon Romanovsky]

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@oracle.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.
> 
> 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.
> gcc online doc didn't help me either.
> 
> To test, I wrote a trivial C program, compiled it with gcc -S and looked at
> the assembly files.
> 
> 
> Maybe, if it is an undefined behavior, other compilers behave differently
> than gcc.
> 
> 
> However, the 2 persons listed bellow have a much better Linux and C
> background than me. So it is likely that my testings were too naive.

There are number of reasons why you didn't notice any difference.
1. {} is GCC extension
2. {} was adopted in latest C standards, so need to check which one GCC 10
is using by default.
3. Main difference will be in padding - {0} will set to zero fields but
won't touch padding, while {} will zero everything.

> 
> 
> Can someone provide some rational or compiler output that confirms that {}
> and {0} are not the same?
> 
> Because if confirmed, I guess that there is some clean-up work to do all
> over the code, not only to please Sparse!
> 
> 
> Thanks in advance.
> CJ
> 
> 
> 
> [1]: Russell King - https://lore.kernel.org/netdev/YRFGxxkNyJDxoGWu@shredder/T/#efe1b6c7862b7ca9588c2734f04be5ef94e03d446
> 
> [2]: Leon Romanovsky - https://lore.kernel.org/netdev/YRFGxxkNyJDxoGWu@shredder/T/#efe1b6c7862b7ca9588c2734f04be5ef94e03d446

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

[Russell King (Oracle)]

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@oracle.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!

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

[Al Viro]

On Sat, Aug 14, 2021 at 03:59:22PM +0200, Christophe JAILLET wrote:

> > +# prefer = {}; to = {0};
> > +		if ($line =~ /= \{ *0 *\}/) {
> > +			WARN("ZERO_INITIALIZER",
> > +			     "= {} is preferred over = {0}\n" . $herecurr);

Sigh...  "is preferred over" by whom?  Use the active voice, would you?

> [1] and [2] state that {} and {0} don't have the same effect. So if correct,
> this is not only a matter of style.
> 
> 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.
> gcc online doc didn't help me either.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf, but empty
initializer-list is gccism anyway.

Section 6.7.8 is the one to look through there.

> Can someone provide some rational or compiler output that confirms that {}
> and {0} are not the same?

Easily: compare
	int x[] = {0};
and
	int x[] = {};

For more obscure example,
	int x = {0};
is valid, if pointless, but
	int x = {};
will be rejected even by gcc.

Incidentally, do *NOT* assume that initializer will do anything with padding
in a structure, no matter how you spell it.  Neither {} nor {0} nor explicit
initializer for each member of struct do anything to the padding.  memset()
does, but anything short of that leaves the padding contents unspecified.

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

[Al Viro]

On Sat, Aug 14, 2021 at 05:38:27PM +0300, Leon Romanovsky wrote:

> There are number of reasons why you didn't notice any difference.
> 1. {} is GCC extension
> 2. {} was adopted in latest C standards, so need to check which one GCC 10
> is using by default.
> 3. Main difference will be in padding - {0} will set to zero fields but
> won't touch padding, while {} will zero everything.

References on (3), please?

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

[Leon Romanovsky]

On Sat, Aug 14, 2021 at 02:57:06PM +0000, Al Viro wrote:
> On Sat, Aug 14, 2021 at 05:38:27PM +0300, Leon Romanovsky wrote:
> 
> > There are number of reasons why you didn't notice any difference.
> > 1. {} is GCC extension
> > 2. {} was adopted in latest C standards, so need to check which one GCC 10
> > is using by default.
> > 3. Main difference will be in padding - {0} will set to zero fields but
> > won't touch padding, while {} will zero everything.
> 
> References on (3), please?

I reread gcc/c/c-typeck.c and at lest for GCC 10, I'm wrong about padding.
Sorry about that.

   8630 struct c_expr
   8631 pop_init_level (location_t loc, int implicit,
   8632                 struct obstack *braced_init_obstack,
   8633                 location_t insert_before)
....
   8692   switch (vec_safe_length (constructor_elements))
   8693     {
   8694     case 0:
   8695       /* Initialization with { } counts as zeroinit.  */
   8696       constructor_zeroinit = 1;
   8697       break;
   8698     case 1:
   8699       /* This might be zeroinit as well.  */
   8700       if (integer_zerop ((*constructor_elements)[0].value))
   8701         constructor_zeroinit = 1;
   8702       break;
   8703     default:
   8704       /* If the constructor has more than one element, it can't be { 0 }.  */
   8705       constructor_zeroinit = 0;
   8706       break;
   8707     }
   8708

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

[Al Viro]

On Sat, Aug 14, 2021 at 06:52:57PM +0300, Leon Romanovsky wrote:

> I reread gcc/c/c-typeck.c and at lest for GCC 10, I'm wrong about padding.
> Sorry about that.
> 
>    8630 struct c_expr
>    8631 pop_init_level (location_t loc, int implicit,
>    8632                 struct obstack *braced_init_obstack,
>    8633                 location_t insert_before)
> ....
>    8692   switch (vec_safe_length (constructor_elements))
>    8693     {
>    8694     case 0:
>    8695       /* Initialization with { } counts as zeroinit.  */
>    8696       constructor_zeroinit = 1;
>    8697       break;
>    8698     case 1:
>    8699       /* This might be zeroinit as well.  */
>    8700       if (integer_zerop ((*constructor_elements)[0].value))
>    8701         constructor_zeroinit = 1;
>    8702       break;
>    8703     default:
>    8704       /* If the constructor has more than one element, it can't be { 0 }.  */
>    8705       constructor_zeroinit = 0;
>    8706       break;
>    8707     }

FWIW, that reads more like "in those cases we might quietly turn the whole
thing into memset()" optimization, with no promise that it will be done
that way in future versions.

And considering the fun effects (infoleaks from stack or heap), it's not
something I'd rely upon - not without a much more explicit promise...

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

[Christophe JAILLET]

Le 14/08/2021 à 16:52, Al Viro a écrit :
> On Sat, Aug 14, 2021 at 03:59:22PM +0200, Christophe JAILLET wrote:
> 
>>> +# prefer = {}; to = {0};
>>> +		if ($line =~ /= \{ *0 *\}/) {
>>> +			WARN("ZERO_INITIALIZER",
>>> +			     "= {} is preferred over = {0}\n" . $herecurr);
> 
> Sigh...  "is preferred over" by whom?  Use the active voice, would you?
> 
>> [1] and [2] state that {} and {0} don't have the same effect. So if correct,
>> this is not only a matter of style.
>>
>> 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.
>> gcc online doc didn't help me either.
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf, but empty
> initializer-list is gccism anyway.
> 
> Section 6.7.8 is the one to look through there.
> 
>> Can someone provide some rational or compiler output that confirms that {}
>> and {0} are not the same?
> 
> Easily: compare
> 	int x[] = {0};
> and
> 	int x[] = {};
> 
> For more obscure example,
> 	int x = {0};
> is valid, if pointless, but
> 	int x = {};
> will be rejected even by gcc.
> 
> Incidentally, do *NOT* assume that initializer will do anything with padding
> in a structure, no matter how you spell it.  Neither {} nor {0} nor explicit
> initializer for each member of struct do anything to the padding.  memset()
> does, but anything short of that leaves the padding contents unspecified.
> 

Thanks for the explanations and exemples.

IIUC, code like [1] may leak some data (1 char) because of the layout of 
'struct atyclk' and we have the erroneous feeling that it is fully 
initialized, because of the "{ 0 }".

Correct?

CJ

[1]: 
https://elixir.bootlin.com/linux/v5.14-rc5/source/drivers/video/fbdev/aty/atyfb_base.c#L1859

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

[Dan Carpenter]

On Sat, Aug 14, 2021 at 02:52:31PM +0000, Al Viro wrote:
> On Sat, Aug 14, 2021 at 03:59:22PM +0200, Christophe JAILLET wrote:
> 
> > > +# prefer = {}; to = {0};
> > > +		if ($line =~ /= \{ *0 *\}/) {
> > > +			WARN("ZERO_INITIALIZER",
> > > +			     "= {} is preferred over = {0}\n" . $herecurr);
> 
> Sigh...  "is preferred over" by whom?  Use the active voice, would you?
> 
> > [1] and [2] state that {} and {0} don't have the same effect. So if correct,
> > this is not only a matter of style.
> > 
> > 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.
> > gcc online doc didn't help me either.
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf, but empty
> initializer-list is gccism anyway.
> 
> Section 6.7.8 is the one to look through there.

That's out of date.  It changed in C11.  Both = { 0 } and = { } will
clear out struct holes. The = { } GCC extension has always initialized
struct holes.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf

For partial initializations then all the padding is zeroed.
Unfortunately if you fully initialize the struct then padding is not
initialized.

regards,
dan carpenter

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

[Russell King (Oracle)]

On Mon, Aug 16, 2021 at 09:55:53AM +0300, Dan Carpenter wrote:
> On Sat, Aug 14, 2021 at 02:52:31PM +0000, Al Viro wrote:
> > On Sat, Aug 14, 2021 at 03:59:22PM +0200, Christophe JAILLET wrote:
> > 
> > > > +# prefer = {}; to = {0};
> > > > +		if ($line =~ /= \{ *0 *\}/) {
> > > > +			WARN("ZERO_INITIALIZER",
> > > > +			     "= {} is preferred over = {0}\n" . $herecurr);
> > 
> > Sigh...  "is preferred over" by whom?  Use the active voice, would you?
> > 
> > > [1] and [2] state that {} and {0} don't have the same effect. So if correct,
> > > this is not only a matter of style.
> > > 
> > > 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.
> > > gcc online doc didn't help me either.
> > 
> > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf, but empty
> > initializer-list is gccism anyway.
> > 
> > Section 6.7.8 is the one to look through there.
> 
> That's out of date.  It changed in C11.  Both = { 0 } and = { } will
> clear out struct holes. The = { } GCC extension has always initialized
> struct holes.
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf
> 
> For partial initializations then all the padding is zeroed.
> Unfortunately if you fully initialize the struct then padding is not
> initialized.

If we're going to discuss which C standard applies to the kernel,
then...

As Kbuild passes -std=gnu89, the kernel expects C89 behaviour with
GNU extensions from the compiler, both C99 and C11 are not that
relevant, although the GNU extensions include some bits from these
standards.

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

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

[Dan Carpenter]

On Mon, Aug 16, 2021 at 08:23:45AM +0100, Russell King (Oracle) wrote:
> > That's out of date.  It changed in C11.  Both = { 0 } and = { } will
> > clear out struct holes. The = { } GCC extension has always initialized
> > struct holes.
> > 
> > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf
> > 
> > For partial initializations then all the padding is zeroed.
> > Unfortunately if you fully initialize the struct then padding is not
> > initialized.
> 
> If we're going to discuss which C standard applies to the kernel,
> then...
> 
> As Kbuild passes -std=gnu89, the kernel expects C89 behaviour with
> GNU extensions from the compiler, both C99 and C11 are not that
> relevant, although the GNU extensions include some bits from these
> standards.

That's fine.  The GCC implementation has always been okay.  The question
is if we could rely on it going forward so now that it's part of the
spec that's very reassuring.

regards,
dan carpenter