[PATCH] libxtables: Dont initialize global xt_params

[PATCH] libxtables: Dont initialize global xt_params

[jamal]

commit 7aad144ecaf7603b17af5372886fe491c3bc6a2f
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date:   Wed Feb 11 16:19:30 2009 -0500

    Dont initialize global xt_params
    To quote Jan Engelhardt <jengelh@medozas.de>
    "
    Do not initialize static members - this takes up extra space
    and adds no benefit. (zeroed anyway even in .bss)
    "
    
    Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

diff --git a/xtables.c b/xtables.c
index 8e28d5e..114a393 100644
--- a/xtables.c
+++ b/xtables.c
@@ -48,7 +48,7 @@
 
 void basic_exit_err(enum xtables_exittype status, const char *msg, ...)
__attribute__((noreturn, format(printf,2,3)));
 
-struct xtables_globals *xt_params = NULL;
+struct xtables_globals *xt_params;
 
 void basic_exit_err(enum xtables_exittype status, const char *msg, ...)
 {

Re: [PATCH] libxtables: Dont initialize global xt_params

[Thomas Jarosch]

Hi Jamal,

On Thursday, 12. February 2009 15:16:02 jamal wrote:
> commit 7aad144ecaf7603b17af5372886fe491c3bc6a2f
> Author: Jamal Hadi Salim <hadi@cyberus.ca>
> Date:   Wed Feb 11 16:19:30 2009 -0500
>
>     Dont initialize global xt_params
>     To quote Jan Engelhardt <jengelh@medozas.de>
>     "
>     Do not initialize static members - this takes up extra space
>     and adds no benefit. (zeroed anyway even in .bss)
>     "
>
>     Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
>
> diff --git a/xtables.c b/xtables.c
> index 8e28d5e..114a393 100644
> --- a/xtables.c
> +++ b/xtables.c
> @@ -48,7 +48,7 @@
>
>  void basic_exit_err(enum xtables_exittype status, const char *msg, ...)
> __attribute__((noreturn, format(printf,2,3)));
>
> -struct xtables_globals *xt_params = NULL;
> +struct xtables_globals *xt_params;

Thanks for your patch. Did you know about zeroing of .bss before?

I see two possible drawbacks using this style:

1. Other people which don't know this "trick" will think
the variable is not initialized -> Hard to read.

2. If that variable gets moved f.e. inside a function, it will become 
uninitialized. Also I'm not sure if the savings are even measurable...

Just my two cents
Thomas

Re: [PATCH] libxtables: Dont initialize global xt_params

[Jan Engelhardt]

On Thursday 2009-02-12 16:09, Thomas Jarosch wrote:
>> -struct xtables_globals *xt_params = NULL;
>> +struct xtables_globals *xt_params;
>
>Thanks for your patch. Did you know about zeroing of .bss before?
>
>I see two possible drawbacks using this style:
>
>1. Other people which don't know this "trick" will think
>the variable is not initialized -> Hard to read.

Well, maybe then they should get a better C book --
automatic initialization is said to be(*) part of the C standard.
And one should know the standard of a language IMHO.

(*) Because it is not free, it remains a saga from people who do
have access to it. :p

>2. If that variable gets moved f.e. inside a function, it will become 
>uninitialized. Also I'm not sure if the savings are even measurable...

It seems to be for the Linux kernel. Especially when you happen
to have large globals (both a lot of them, and large; e.g.
foo[NR_CUPS]) this becomes a concern.


Just my 2 ints. (Oh wait, move them to bss!)

>
>Just my two cents
>Thomas
>

Re: [PATCH] libxtables: Dont initialize global xt_params

[Thomas Jarosch]

On Thursday, 12. February 2009 16:14:23 Jan Engelhardt wrote:
> >1. Other people which don't know this "trick" will think
> >the variable is not initialized -> Hard to read.
>
> Well, maybe then they should get a better C book --
> automatic initialization is said to be(*) part of the C standard.
> And one should know the standard of a language IMHO.
>
> (*) Because it is not free, it remains a saga from people who do
> have access to it. :p

That is completly true. OTOH avoid the dark corners of the language.

> >2. If that variable gets moved f.e. inside a function, it will become
> >uninitialized. Also I'm not sure if the savings are even measurable...
>
> It seems to be for the Linux kernel. Especially when you happen
> to have large globals (both a lot of them, and large; e.g.
> foo[NR_CUPS]) this becomes a concern.

Well, I guess that's a job for the compiler/optimizer. I did a quick test by 
writing two versions of a small program initializing a static variable with 
zero and one version that doesn't (=zeroed in .bss). Guess what,
the size of the resulting executable stays the same.

When I initialize the variable with a non-zero value, then the program size 
increases. I tested "-O2", "-O0" and "-Os" and the results where the same.
Feel free to look at the assembler output, though I guess this optimization
is not measurable and makes the code harder to read :o)

Thomas

[PATCH] libxtables: Dont initialize global xt_params

[jamal]

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


This is a resend with attachment instead of inlining.

cheers,
jamal

[-- Attachment #2: iptv2-fix01 --]
[-- Type: text/plain, Size: 804 bytes --]

commit 7aad144ecaf7603b17af5372886fe491c3bc6a2f
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date:   Wed Feb 11 16:19:30 2009 -0500

    Dont initialize global xt_params
    To quote Jan Engelhardt <jengelh@medozas.de>
    "
    Do not initialize static members - this takes up extra space
    and adds no benefit. (zeroed anyway even in .bss)
    "
    
    Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

diff --git a/xtables.c b/xtables.c
index 8e28d5e..114a393 100644
--- a/xtables.c
+++ b/xtables.c
@@ -48,7 +48,7 @@
 
 void basic_exit_err(enum xtables_exittype status, const char *msg, ...) __attribute__((noreturn, format(printf,2,3)));
 
-struct xtables_globals *xt_params = NULL;
+struct xtables_globals *xt_params;
 
 void basic_exit_err(enum xtables_exittype status, const char *msg, ...)
 {

Re: [PATCH] libxtables: Dont initialize global xt_params

[jamal]

On Thu, 2009-02-12 at 16:41 +0100, Thomas Jarosch wrote:

> Well, I guess that's a job for the compiler/optimizer. I did a quick test by 
> writing two versions of a small program initializing a static variable with 
> zero and one version that doesn't (=zeroed in .bss). Guess what,
> the size of the resulting executable stays the same.
> 
> When I initialize the variable with a non-zero value, then the program size 
> increases. I tested "-O2", "-O0" and "-Os" and the results where the same.
> Feel free to look at the assembler output, though I guess this optimization
> is not measurable and makes the code harder to read :o)
> 

I have no objection with junking the patch i sent. My initial intent of
NULLing was to use it as a check when first initializing - but now i
realize it actually didnt matter; and the readability arguement sounds
fair.

cheers,
jamal

Re: [PATCH] libxtables: Dont initialize global xt_params

[Thomas Jarosch]

jamal wrote:
> I have no objection with junking the patch i sent. My initial intent of
> NULLing was to use it as a check when first initializing - but now i
> realize it actually didnt matter; and the readability arguement sounds
> fair.

Thanks and have a nice weekend :-)

Thomas

Re: [PATCH] libxtables: Dont initialize global xt_params

[Philip Craig]

Thomas Jarosch wrote:
> Well, I guess that's a job for the compiler/optimizer. I did a quick test by 
> writing two versions of a small program initializing a static variable with 
> zero and one version that doesn't (=zeroed in .bss). Guess what,
> the size of the resulting executable stays the same.
> 
> When I initialize the variable with a non-zero value, then the program size 
> increases. I tested "-O2", "-O0" and "-Os" and the results where the same.
> Feel free to look at the assembler output, though I guess this optimization
> is not measurable and makes the code harder to read :o)

For gcc, this depends on the -fno-zero-initialized-in-bss option.
Recommendations to avoid zero initialization generally come from
a time when gcc didn't do this by default.  Now it is more just
personal preference.

Re: [PATCH] libxtables: Dont initialize global xt_params

[Patrick McHardy]

Philip Craig wrote:
> Thomas Jarosch wrote:
>> Well, I guess that's a job for the compiler/optimizer. I did a quick test by 
>> writing two versions of a small program initializing a static variable with 
>> zero and one version that doesn't (=zeroed in .bss). Guess what,
>> the size of the resulting executable stays the same.
>>
>> When I initialize the variable with a non-zero value, then the program size 
>> increases. I tested "-O2", "-O0" and "-Os" and the results where the same.
>> Feel free to look at the assembler output, though I guess this optimization
>> is not measurable and makes the code harder to read :o)
> 
> For gcc, this depends on the -fno-zero-initialized-in-bss option.
> Recommendations to avoid zero initialization generally come from
> a time when gcc didn't do this by default.  Now it is more just
> personal preference.

I think the 3.x versions don't do this by default, so as long as they're
supported by the kernel, we should expect people to use them and not
assume defaults of later versions.

Re: [PATCH] libxtables: Dont initialize global xt_params

[Thomas Jarosch]

On Monday, 16. February 2009 11:38:49 you wrote:
> >> Well, I guess that's a job for the compiler/optimizer. I did a quick
> >> test by writing two versions of a small program initializing a static
> >> variable with zero and one version that doesn't (=zeroed in .bss). Guess
> >> what, the size of the resulting executable stays the same.
> >>
> >> When I initialize the variable with a non-zero value, then the program
> >> size increases. I tested "-O2", "-O0" and "-Os" and the results where
> >> the same. Feel free to look at the assembler output, though I guess this
> >> optimization is not measurable and makes the code harder to read :o)
> >
> > For gcc, this depends on the -fno-zero-initialized-in-bss option.
> > Recommendations to avoid zero initialization generally come from
> > a time when gcc didn't do this by default.  Now it is more just
> > personal preference.
>
> I think the 3.x versions don't do this by default, so as long as they're
> supported by the kernel, we should expect people to use them and not
> assume defaults of later versions.

Thanks for clearing this up, Philip and Patrick.

So we've got so far:

1. Better readbility by writing "xyz = NULL;"
2. Correct and a tiny bit larger code using gcc 3
3. Correct and compact code using gcc 4 (released April 2005).

I guess the assembler code generated from gcc 3 <-> gcc 4
will have other performance/size differences,
but it's Patrick's final decision what to do :-)

Cheers,
Thomas

Re: [PATCH] libxtables: Dont initialize global xt_params

[Patrick McHardy]

Thomas Jarosch wrote:
> On Monday, 16. February 2009 11:38:49 you wrote:
>> I think the 3.x versions don't do this by default, so as long as they're
>> supported by the kernel, we should expect people to use them and not
>> assume defaults of later versions.
> 
> Thanks for clearing this up, Philip and Patrick.
> 
> So we've got so far:
> 
> 1. Better readbility by writing "xyz = NULL;"
> 2. Correct and a tiny bit larger code using gcc 3
> 3. Correct and compact code using gcc 4 (released April 2005).
> 
> I guess the assembler code generated from gcc 3 <-> gcc 4
> will have other performance/size differences,
> but it's Patrick's final decision what to do :-)

I actually prefer no initializers for two reasons:

- it avoids these discussions that come up once every couple
   of months :)

- they're useless