tasklwIP - A Lightweight TCP/IP stack - Tasks: task #7054, Clarification needed for variable...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #7054: Clarification needed for variable initialization

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Tue 03 Jul 2007 07:43:45 PM UTC
   
 
Category:  None Should Start On:  Tue 03 Jul 2007 12:00:00 AM UTC
Should be Finished on:  Tue 03 Jul 2007 12:00:00 AM UTC Priority:  3 - Low
Status:  Done Privacy:  Public
Assigned to:  goldsimon Percent Complete:  100%
Open/Closed:  Closed Planned Release:  None
Effort:  0.00

Jump to the original submission

Tue 09 Oct 2007 08:01:06 PM UTC, comment #25: 

Checked in. Closing this as done.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 09 Oct 2007 07:44:28 PM UTC, comment #24: 

OK, I'll do it, then!

Simon Goldschmidt <goldsimon>
Group administrator
Sat 06 Oct 2007 12:07:17 PM UTC, comment #23: 


>-sockets.c: I would write :
>+static sys_sem_t socksem;
>+static sys_sem_t selectsem;
>(see comments #15 & 16)


Thanks, I had that in mind but didn't find where we talked about it ;-)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Oct 2007 03:40:10 PM UTC, comment #22: 

About the patch:

-contrib.txt: ok

-sockets.c: I would write :
+static sys_sem_t socksem;
+static sys_sem_t selectsem;
(see comments #15 & 16)

-tcpip.c: ok
-mem.c: not yet checked
-netif.c: why let explicit init here (for netif_list & netif_default)?
-raw.c: ok
-stats.c: ok
-tcp.c: not yet checked
-udp.c: ok
-igmp.c: ok
-ip.c: ok
-ip_frag.c: not yet checked
-mib_structs.c: ok
-msg_in.c: ok
-ip.h: ok
-netif.h: ok
-raw.h: ok
-stats.h: ok
-tcp.h: ok
-udp.h: ok
-etharp.h: ok
-etharp.c: not yet checked


Frédéric Bernon <fbernon>
Group Member
Thu 04 Oct 2007 07:07:18 PM UTC, comment #21: 

I've put together a patch that removes the need for nearly all _init() functions (since they all have set values to zero/NULL that are set by the compiler already.

Kieran, please comment, so I know if this is the right direction for you.

(file #14083)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 24 Aug 2007 04:05:15 PM UTC, comment #20: 

I was being more general to any _init routine.  For example, say one wants to start/stop dhcp.  (I have not looked at the dhcp code, I am just giving a example of something that you may want to start/stop)

The name _init may give a developer the idea that it "reinitializes" where it may not really achieve that.

I have no problem using the compiler to zero values, I was just pointing out that there are cases where one might want to explicitly zero a value to allow "re-init" and we may want make sure we comment these so they are not deleted later by someone else.

Bill Florac <bflorac>
Fri 24 Aug 2007 03:44:43 PM UTC, comment #19: 

Why? Why would you need to stop and restart the whole stack? This is a very improbable thing for an embedded device to do. 99% of people wouldn't need it - once the stack is going they leave it. If they really must stop the stack, then they can bring down the interfaces. They shouldn't try and call the stack init functions to get it going again.

Jonathan Larmour <jifl>
Group Member
Fri 24 Aug 2007 03:32:43 PM UTC, comment #18: 

If we rely on the compiler, than we need to make sure that on any processes that can stopped and restarted, any initializing needed to restart is done in the _start routine. Calling _init will not put everything thing back to "startup" values. 

Of course if there are such requirement, one can still set the values to zero in the _init and comment them as needed.

Bill Florac <bflorac>
Fri 24 Aug 2007 02:45:01 PM UTC, comment #17: 

I'd agree with the consensus that seems to have been reached here, i.e. we can assume that a compiler will initialise static variables and globals to zero, and so INIT_NULL/ZERO should not be necessary, nor should it be necessary to later set them to zero in an _init function.

However, don't go to a lot of effort to do this now as it is just general cleanup, but feel free to make things more standard across the code when doing related changes.

Kieran Mansley <kieranm>
Group Member
Tue 31 Jul 2007 12:26:05 AM UTC, comment #16: 

Ah yes Frédéric, good point!

Jonathan Larmour <jifl>
Group Member
Mon 30 Jul 2007 10:29:52 PM UTC, comment #15: 


>Re comment #5, it is still better to initialise socksem and selectsem to SYS_SEM_NULL at the time they are declared - that way the loader initialises it along with other initialised data, which means less code.


If we consider that lwip_socket_init is now mandatory, socksem & selectsem can even have no explicit initialization at the time they are declared, since their first values will be set by the two sys_sem_new calls...

Frédéric Bernon <fbernon>
Group Member
Mon 30 Jul 2007 09:29:17 PM UTC, comment #14: 

I'm with David Empson. Compilers/linkers have zero initialised statics and globals to 0 since the days of K&R C, the ANSI C/ISO C  standards of 1989/1990 made it concrete. We really shouldn't need to worry about that any more. It's more likely that someone (maybe the user, not the compiler or OS writers) has a bug in their linker script.

At the same time, there is almost certainly no harm in explicit zero initialisation. Compilers should still put these in the BSS, rather than the initialised data section. But there's no standard governing this - in practice it's most likely though.

Re comment #5, it is still better to initialise socksem and selectsem to SYS_SEM_NULL at the time they are declared - that way the loader initialises it along with other initialised data, which means less code.

Personally, I wouldn't support broken compilers.

But we should still remove all the places that a _init() function sets a static/global to 0. e.g. mem_init, and I'm sure others.

Jonathan Larmour <jifl>
Group Member
Thu 05 Jul 2007 11:57:35 PM UTC, comment #13: 

ISO/IEC C99 support is patchy at present. None of the embedded compilers I use are anywhere near being compliant with it.

I think it is reasonable to expect a high degree of ANSI/ISO C90 compliance, and LWIP should not be bending over backwards to cope with compilers that aren't ANSI compatible at all.

LWIP should be avoiding features which require C99 or extensions to the C90 standard (such as // comments).

In my opinion, a compiler which doesn't set static variables to zero on startup is seriously broken, as it doesn't comply with any C standard.

GCC is reasonably compliant with C90 (plus extensions) and implements several features of C99, so I wouldn't call it "broken".

My concern with the INIT_NULL solution is not its effect on the object code, but on the readability of the source code. Several variables using INIT_NULL in one area of a source file will look ugly.

If this proposal is carried through, I'd also suggest INIT_ZERO as a better name, since INIT_NULL implies a pointer. Perhaps both should be used: INIT_ZERO for arithmetic variables, INIT_NULL for pointer variables. This also allows for compilers which define NULL as something "better" than (void *)0.

Arrays could use INIT_ARRAY_ZERO, but structures and unions need the same macro, so a more generic name would be better. INIT_AGGREGATE_ZERO is the best choice but is too long.

I'm interested to note that the C90 standard allows a scalar to be initialized with a brace-enclosed expression, e.g.

int a = {0};

I've never tried this and some compilers might not like it. If it is supported by all compilers, then INIT_ZERO can be declared as

#define INIT_ZERO = {0}

(or blank if not required) and this will work with variables of all types.

David Empson <dempson>
Thu 05 Jul 2007 04:54:56 PM UTC, comment #12: 

I didn't have participate to bug #20254. But like I say in comment #10, if we don't support "broken" compilers, so, we don't have to support gcc !!! (and of course, it could seems crazy to do that).

If we tell that we only support ANSI C99 compliant compilers, so, we could tell that "inline functions" is a feature we can used in lwIP source code (since it's in the standard!!!), but in this case, bye-bye gcc users (see http://gcc.gnu.org/gcc-4.2/c99status.html).

I think that your INIT_NULL is a little change which help some users (and it doesn't change code for people who don't have this problem). If the change was a big one, my point of view would be different, but in this case...

Frédéric Bernon <fbernon>
Group Member
Thu 05 Jul 2007 04:43:12 PM UTC, comment #11: 

Hmmm, now that I know it's in the standard, it would indeed be coding around broken compilers, and I thought we had decided not to do that in bug #20254?

Simon Goldschmidt <goldsimon>
Group administrator
Thu 05 Jul 2007 07:19:48 AM UTC, comment #10: 

But even like this, I think the INIT_NULL solution is not so bad, since we only use it on "some" variables.

I'm agree it's a workaround for "broken" compilers. But I'm not sure than even gcc is really at 100% ANSI C99 compliant (and if I read http://gcc.gnu.org/gcc-4.2/c99status.html, I think we can tell is not). Most of embedded compiler are provided by hardware manufacturer, and I'm not sure that all are C99 compliant.

So, since adding this feature doesn't change the code if you define it to "empty", I think we can help users having "broken" compilers with these "pretty small" changes. After all, having the most portable code as possible is one of the goal of lwIP, right?


Frédéric Bernon <fbernon>
Group Member
Thu 05 Jul 2007 07:09:52 AM UTC, comment #9: 

Same thing in recent relase "ISO/IEC 9899:1999 - Programming languages - C" from JTC 1/SC 22/WG 14 :

6.7.8 Initialization

If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static storage duration is not initialized explicitly, then:

— if it has pointer type, it is initialized to a null pointer;
— if it has arithmetic type, it is initialized to (positive or unsigned) zero;
— if it is an aggregate, every member is initialized (recursively) according to these rules;
— if it is a union, the first named member is initialized (recursively) according to these
rules.

Frédéric Bernon <fbernon>
Group Member
Thu 05 Jul 2007 06:49:09 AM UTC, comment #8: 

Yes, I am blind. Thanks for clarification!

Simon Goldschmidt <goldsimon>
Group administrator
Thu 05 Jul 2007 03:48:01 AM UTC, comment #7: 

In e-mail discussion on lwip-devel (outside the bug tracker), Simon Goldschmidt wrote:

> Maybe I'm blind, I didn't find it in the ANSI C standard :-(


Looking at my copy of the original ANSI C standard (1990) it is specified in section 6.5.7 Initialization, under the Semantics sub-heading:

"If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static storage duration is not initialized explicitly, it is initialized implicitly as if every member that has arithmetic type were assigned 0 and every member that has pointer type were assigned null pointer constant."

i.e. it is a requirement of ANSI C. A compiler must do this if set to operate in ANSI mode. It might decide to omit it if running in non-ANSI mode (but should at least offer it as an option).

In my experience with embedded compilers, it is usually a function of the startup code, and if the user has the source code to this, they could modify it to omit clearing of uninitialized variables, e.g. for performance reasons.

I think it is reasonable to require implicit initialization to zero of static variables for LWIP. I'd prefer that to an ugly solution like INIT_NULL scattered everywhere in the source code.

A user who really cares about the performance implications should do something like arrange for the LWIP uninitialized variables to be linked in a memory area which is cleared on startup, while their "large and speed critical" uninitialized variables go in a separate memory area. There could be performance issues with this scheme if LWIP has a lot of large static variables.

David Empson <dempson>
Wed 04 Jul 2007 02:42:38 PM UTC, comment #6: 

http://savannah.nongnu.org/bugs/?20254

(Robert Ammerman wrote: )

"It is indeed part of the C standard that global variables without explicit
initializers are set to zero.

However, talk about BSS or INITDATA is referring to some, but not all
compilers. The way the compiler manages the initialization is not defined.
For example, it would be perfectly valid for the compiler/linker to generate
a bunch of assignment and "memset" type statements that are executed before
main()"

Frédéric Bernon <fbernon>
Group Member
Wed 04 Jul 2007 09:38:03 AM UTC, comment #5: 

Some problem with some init. In sockets.c file, you could see :

static sys_sem_t socksem = 0;
static sys_sem_t selectsem = 0;

But it's not good, since 0 is not the same as SYS_SEM_NULL: this define is provide in each sys_arch.h to determine which is the invalid value for a semaphore (see sys_arch.txt). In my case, I got...

#define SYS_SEM_NULL 0xffffffff

Since my semaphore subsystem handler is based on a array (where 0 is a valid value).

Hopefully, they are - in CVS HEAD - initialized in lwip_socket_init()...

Frédéric Bernon <fbernon>
Group Member
Wed 04 Jul 2007 08:09:01 AM UTC, comment #4: 

("The C Programming Language" by Kernighan & Ritchie)

4.9 Initialization

Initialization has been mentioned in passing many times so far, but always peripherally to some other topic. This section summarizes some of the rules, now that we have discussed the various storage classes.

In the absence of explicit initialization, external and static variables are guaranteed to be initialized to zero; automatic and register variables have undefined (i.e., garbage) initial values.

Scalar variables may be initialized when they are defined, by following the name with an equals sign and an expression:

  int x = 1;
  char squota = '\'';
  long day = 1000L 60L 60L 24L; / milliseconds/day */

For external and static variables, the initializer must be a constant expression; the initialization is done once, conceptionally before the program begins execution. For automatic and register variables, the initializer is not restricted to being a constant: it may be any expression involving previously defined values, even function calls. For example, the initialization of the binary search program in Section 3.3 could be written as

int binsearch(int x, int v[], int n)
{
  int low = 0;
  int high = n - 1;
  73
  int mid;
  ...
}

instead of

  int low, high, mid;
  low = 0;
  high = n - 1;

In effect, initialization of automatic variables are just shorthand for assignment statements. Which form to prefer is largely a matter of taste. We have generally used explicit assignments, because initializers in declarations are harder to see and further away from the point of use.

An array may be initialized by following its declaration with a list of initializers enclosed in braces and separated by commas. For example, to initialize an array days with the number of
days in each month:

int days[] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }

When the size of the array is omitted, the compiler will compute the length by counting the initializers, of which there are 12 in this case.

If there are fewer initializers for an array than the specified size, the others will be zero for external, static and automatic variables. It is an error to have too many initializers. There is no way to specify repetition of an initializer, nor to initialize an element in the middle of an array without supplying all the preceding values as well.

Character arrays are a special case of initialization; a string may be used instead of the braces and commas notation:

  char pattern = "ould";

is a shorthand for the longer but equivalent

  char pattern[] = { 'o', 'u', 'l', 'd', '\0' };

In this case, the array size is five (four characters plus the terminating '\0').

Frédéric Bernon <fbernon>
Group Member
Wed 04 Jul 2007 07:58:20 AM UTC, comment #3: 

In "The C Programming Language" by Kernighan & Ritchie, we can read "External and static variables are initialized to zero by default." Of course, you need to have a ANSI C compiler to be sure of that.

Problem with that is you have to be sure your compiler is really ANSI. So, using _init functions the only solution to be sure about the init value for any compiler.

Using INIT_NULL is a good idea since each one can decide to belive or not the ANSI compilance of his compiler. For array like tcp_backoff, I  think you can let them like this. For ZEROing arrays, you could do the same thing that INIT_NULL like:

#define INIT_ARRAY_NULL = {0}


Frédéric Bernon <fbernon>
Group Member
Wed 04 Jul 2007 07:38:29 AM UTC, comment #2: 


> (Julian Gardner wrote)


Hehe.

> This is the way it should work, i think setting a variable to 0 is by default the norm.


You think. I do, too. But I'd like to know before I tell people their compiler is broken!

> As to the tcp_backoff this will make the code larger as if
> taking the tcp_backoff as an example we would have 13 stores
> and at least 7 loads.
>


Of course it will get bigger. It's not my intention to do that. I was referring to ZEROing arrays that are NOT initialized, and I'd use memset for that. If we make it mandatory for compilers to zero the bss section, we can leave that away. But right now, we don't seem to know that for sure!

Simon Goldschmidt <goldsimon>
Group administrator
Wed 04 Jul 2007 07:30:57 AM UTC, comment #1: 

(Julian Gardner wrote)

Well when i start using any code i remove all 0 initialisation code, so this forces the compiler to stick the variable in the BSS section.

This is the way it should work, i think setting a variable to 0 is by default the norm.

As to the tcp_backoff this will make the code larger as if taking the tcp_backoff as an example we would have 13 stores and at least 7 loads.

leave the code as is and start removing the NULL and 0 assignments for GLOBAL variables

Frédéric Bernon <fbernon>
Group Member
Tue 03 Jul 2007 07:43:45 PM UTC, original submission:  

The current code mixes:
- initialization to zero for global (and global static) variables
- uninitialized static arrays (initialized to 0 by compiler/loader)
- initialization to other values for global variables
- initialization in _init() functions

I'd favour making it the same for all files:
- always initialize non-zero global variables at declaration (e.g. tcp_backoff array)
- create define to initialize global variables to zero or not (decides which section they are put into and if they take place in the binary before execution) (e.g. 'struct tcp_pcb *tcp_tw_pcbs INIT_NULL;' and '#define INIT_NULL = 0' or '#define INIT_NULL') (there is a downside for arrays, so arrays that should be zeroed - like sockets[NUM_SOCKETS] - would have to be memset to NULL in an init function)
- generally don't zero variables in init functions that are already zeroed on declaration

Any comments or preferences?

Simon Goldschmidt <goldsimon>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #14083:  lwip_init.patch added by goldsimon (15KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bflorac (Posted a comment)
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by kieranm (Updated the item)
  • -email is unavailable- added by dempson (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by goldsimon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-10-09 goldsimon StatusNone Done
        Percent Complete0% 100%
        Open/ClosedOpen Closed
    2007-10-06 goldsimon Assigned toNone goldsimon
    2007-10-04 goldsimon Attached File- Added lwip_init.patch, #14083
    2007-07-13 kieranm Priority5 - Normal 3 - Low

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code