drop overzealous ERROR: do not initialise statics to 0 or NULL from checkpatch.pl

drop overzealous ERROR: do not initialise statics to 0 or NULL from checkpatch.pl

[John Kacur]

Could we drop this somewhat overzealous "ERROR: do not initialise
statics to 0 or NULL" from checkpatch.pl?

Reasoning:
1. This is not part of Documentation/CodingStyle
2. K&R 2nd.ed do it (pg 83, static int bufp = 0;) The purpose is to
remove access to the bufp from external routines, and to avoid name
conflict)
3. It can be a good form of documentation.
4. It creates a lot of needless code churn to change this kind of
thing for no good reason.
5. It doesn't even change the object size (thus kernel size) to do so.
Demo with user space code.

jkacur@linux-ipxk:~/try> cat foo.c
#include <stdio.h>
#include <stdlib.h>

static int a[1000];

/* Function Prototype */
void foo(void);
int main(void)
{
        exit(0);
}

void foo(void)
{
        static int b[1000];
        static int c;
}
jkacur@linux-ipxk:~/try> gcc foo.c
jkacur@linux-ipxk:~/try> size a.out
   text    data     bss     dec     hex filename
   1203     520    8064    9787    263b a.out
jkacur@linux-ipxk:~/try> ls -l a.out
-rwxr-xr-x 1 jkacur users 11237 2008-08-13 12:26 a.out

Now initialize all the statics to 0 and there will be no difference in
the object size
jkacur@linux-ipxk:~/try> cat foo.c
#include <stdio.h>
#include <stdlib.h>

static int a[1000] = {0};

/* Function Prototype */
void foo(void);
int main(void)
{
        exit(0);
}

void foo(void)
{
        static int b[1000] = {0};
        static int c = 0;
}
jkacur@linux-ipxk:~/try> gcc foo.c
jkacur@linux-ipxk:~/try> size a.out
   text    data     bss     dec     hex filename
   1203     520    8064    9787    263b a.out
<----------------------- No difference with the initialization to 0!!!
jkacur@linux-ipxk:~/try> ls -l a.out
-rwxr-xr-x 1 jkacur users 11237 2008-08-13 12:26 a.out
<----------------------- No difference with the initialization to 0!!!


Now if we initialize it to a value other than 0 or NULL, then the bss
is decreased at the expense of the data section, which does indeed
increase the object size, however checkpatch.pl doesn't complain about
this. (it is valid to do this)
jkacur@linux-ipxk:~/try> cat foo.c
#include <stdio.h>
#include <stdlib.h>

static int a[1000] = {1};

/* Function Prototype */
void foo(void);
int main(void)
{
        exit(0);
}

void foo(void)
{
        static int b[1000] = {1};
        static int c = 1;
}
jkacur@linux-ipxk:~/try> gcc foo.c
jkacur@linux-ipxk:~/try> size a.out
   text    data     bss     dec     hex filename
   1203    8568      16    9787    263b a.out
jkacur@linux-ipxk:~/try> ls -l a.out
-rwxr-xr-x 1 jkacur users 19301 2008-08-13 12:27 a.out

Re: drop overzealous ERROR: do not initialise statics to 0 or NULL from checkpatch.pl

[John Kacur]

[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]

On Wed, Aug 13, 2008 at 12:39 PM, John Kacur <jkacur@gmail.com> wrote:
> Could we drop this somewhat overzealous "ERROR: do not initialise
> statics to 0 or NULL" from checkpatch.pl?
>
> Reasoning:
> 1. This is not part of Documentation/CodingStyle
> 2. K&R 2nd.ed do it (pg 83, static int bufp = 0;) The purpose is to
> remove access to the bufp from external routines, and to avoid name
> conflict)
> 3. It can be a good form of documentation.
> 4. It creates a lot of needless code churn to change this kind of
> thing for no good reason.
> 5. It doesn't even change the object size (thus kernel size) to do so.
> Demo with user space code.
>
> jkacur@linux-ipxk:~/try> cat foo.c
> #include <stdio.h>
> #include <stdlib.h>
>
> static int a[1000];
>
> /* Function Prototype */
> void foo(void);
> int main(void)
> {
>        exit(0);
> }
>
> void foo(void)
> {
>        static int b[1000];
>        static int c;
> }
> jkacur@linux-ipxk:~/try> gcc foo.c
> jkacur@linux-ipxk:~/try> size a.out
>   text    data     bss     dec     hex filename
>   1203     520    8064    9787    263b a.out
> jkacur@linux-ipxk:~/try> ls -l a.out
> -rwxr-xr-x 1 jkacur users 11237 2008-08-13 12:26 a.out
>
> Now initialize all the statics to 0 and there will be no difference in
> the object size
> jkacur@linux-ipxk:~/try> cat foo.c
> #include <stdio.h>
> #include <stdlib.h>
>
> static int a[1000] = {0};
>
> /* Function Prototype */
> void foo(void);
> int main(void)
> {
>        exit(0);
> }
>
> void foo(void)
> {
>        static int b[1000] = {0};
>        static int c = 0;
> }
> jkacur@linux-ipxk:~/try> gcc foo.c
> jkacur@linux-ipxk:~/try> size a.out
>   text    data     bss     dec     hex filename
>   1203     520    8064    9787    263b a.out
> <----------------------- No difference with the initialization to 0!!!
> jkacur@linux-ipxk:~/try> ls -l a.out
> -rwxr-xr-x 1 jkacur users 11237 2008-08-13 12:26 a.out
> <----------------------- No difference with the initialization to 0!!!
>
>
> Now if we initialize it to a value other than 0 or NULL, then the bss
> is decreased at the expense of the data section, which does indeed
> increase the object size, however checkpatch.pl doesn't complain about
> this. (it is valid to do this)
> jkacur@linux-ipxk:~/try> cat foo.c
> #include <stdio.h>
> #include <stdlib.h>
>
> static int a[1000] = {1};
>
> /* Function Prototype */
> void foo(void);
> int main(void)
> {
>        exit(0);
> }
>
> void foo(void)
> {
>        static int b[1000] = {1};
>        static int c = 1;
> }
> jkacur@linux-ipxk:~/try> gcc foo.c
> jkacur@linux-ipxk:~/try> size a.out
>   text    data     bss     dec     hex filename
>   1203    8568      16    9787    263b a.out
> jkacur@linux-ipxk:~/try> ls -l a.out
> -rwxr-xr-x 1 jkacur users 19301 2008-08-13 12:27 a.out
>

My apologies for not ccing the maintainers of checkpatch the first
time. Attached is a patch to remove the check in case anybody agrees
with me. :)
The patch is against a recently updated git tree.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: remove_static_initialise_error.patch --]
[-- Type: text/x-patch; name=remove_static_initialise_error.patch, Size: 688 bytes --]

Subject: Remove unnecessary error about initialising statics to 0 or NULL

Signed-off-by: John Kacur <jkacur at gmail dot com>

Index: linux-2.6/scripts/checkpatch.pl
===================================================================
--- linux-2.6.orig/scripts/checkpatch.pl
+++ linux-2.6/scripts/checkpatch.pl
@@ -1341,11 +1341,6 @@ sub process {
 			ERROR("do not initialise externals to 0 or NULL\n" .
 				$herecurr);
 		}
-# check for static initialisers.
-		if ($line =~ /\s*static\s.*=\s*(0|NULL|false)\s*;/) {
-			ERROR("do not initialise statics to 0 or NULL\n" .
-				$herecurr);
-		}
 
 # check for new typedefs, only function parameters and sparse annotations
 # make sense.

Re: drop overzealous ERROR: do not initialise statics to 0 or NULL from checkpatch.pl

[Arnd Bergmann]

On Wednesday 13 August 2008, John Kacur wrote:
> Could we drop this somewhat overzealous "ERROR: do not initialise
> statics to 0 or NULL" from checkpatch.pl?

I totally agree we should drop it.

> Reasoning:
> 1. This is not part of Documentation/CodingStyle

However, it is in http://kernel-janitor.sourceforge.net/TODO and should
be dropped from there as well.

> 3. It can be a good form of documentation.

I don't think so. Every C programmer should know that it is the
same. 

> 5. It doesn't even change the object size (thus kernel size) to do so.
> Demo with user space code.

I'm not sure what the last compiler version was on which it made the
difference, probably 2.95 or 3.0 or something else that is no longer
supported.
I do remember that at some point in time, we could save a few bytes by
doing it.

	Arnd <><

Re: drop overzealous ERROR: do not initialise statics to 0 or NULL from checkpatch.pl

[Guennadi Liakhovetski]

On Wed, 13 Aug 2008, John Kacur wrote:

> Could we drop this somewhat overzealous "ERROR: do not initialise
> statics to 0 or NULL" from checkpatch.pl?
> 
> Reasoning:
> 1. This is not part of Documentation/CodingStyle
> 2. K&R 2nd.ed do it (pg 83, static int bufp = 0;) The purpose is to
> remove access to the bufp from external routines, and to avoid name
> conflict)

No, "static" "removes access to the bufp from external routines, and 
avoids name conflict", not the initialization to 0.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: drop overzealous ERROR: do not initialise statics to 0 or NULL from checkpatch.pl

[John Kacur]

On Wed, Aug 13, 2008 at 10:19 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Wed, 13 Aug 2008, John Kacur wrote:
>
>> Could we drop this somewhat overzealous "ERROR: do not initialise
>> statics to 0 or NULL" from checkpatch.pl?
>>
>> Reasoning:
>> 1. This is not part of Documentation/CodingStyle
>> 2. K&R 2nd.ed do it (pg 83, static int bufp = 0;) The purpose is to
>> remove access to the bufp from external routines, and to avoid name
>> conflict)
>
> No, "static" "removes access to the bufp from external routines, and
> avoids name conflict", not the initialization to 0.
>

That is true, but the point is that even the folks who invented the
language don't have a problem with making the initialization explicit.
I'm not even trying to argue that folks should do it one way or the
other, I'm just saying it is unimportant, so let's drop it from
checkpatch.pl and save ourselves a lot of pointless code churn.
btw, see pg 94 of "The Practice of Programming" where the masters also
explicitly initialize statics.