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
On Wed, Aug 13, 2008 at 12:39 PM, John Kacur <[email protected]> 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.
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 <><
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
On Wed, Aug 13, 2008 at 10:19 PM, Guennadi Liakhovetski
<[email protected]> 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.