Variable Initialisation Considered Harmful ? 
Author Message
 Variable Initialisation Considered Harmful ?

Hi,
I was writing some code the other day. The gist of it was this.......

void * some_func( void )
        {
        void * ptr;

        ptr = NULL;       /* NOTE THIS LINE */

        if (condition1)
                /* code1 */
        else if (condition2)    
                * code2 */
        /* more else if's */

        return ptr;
        }

The code was clean through lint, but had a bug (which I caught using a test barrage.
Its great when a test barrage finds a bug.) The bug in this case was that I had omiited
to assign a value to a ptr within one of the if (condition) blocks.
I fixed the bug, and then (as I always do in such cases, and more importantly) I tried to find out if
        a) I could have automatically detected the bug
        b) I can automatically detect such bugs in the future.

It turns out that I can
The answer is NOT to initialise the ptr to default value. That way, when you lint it,
if there is a data flow path that does not initialise ptr, lint will warn you.

This seems to fly in the face of all the "text-book" advice.

Any comments?
Cheers
JJ



Sun, 18 May 1997 20:06:26 GMT  
 Variable Initialisation Considered Harmful ?

Quote:

>It turns out that I can
>The answer is NOT to initialise the ptr to default value. That way, when you
> lint it,
>if there is a data flow path that does not initialise ptr, lint will warn you.

>This seems to fly in the face of all the "text-book" advice.

There is no point in initialising variables twice - if you know a variable is
always assigned a value before it is read there is no point in initialising
it again at the start of the function. The natural conclusion to that would
be initialising every variable defined in a function at its definition which
would be a mess.

There are a few cases (e.g. variable controlled state machines) where the
compiler can't detect this correctly and can generate warnings for correct
code. Since I prefer to keep warnings enabled I initialise the affected
variables to shut the compiler up. This is rare though.

I've never come across a text book that suggests this - perhaps you could
tell us where you saw it and what it actually said?

--
-----------------------------------------


-----------------------------------------



Sun, 18 May 1997 21:44:28 GMT  
 Variable Initialisation Considered Harmful ?

Quote:


>>It turns out that I can
>>The answer is NOT to initialise the ptr to default value.
>>   ----->8-------8<--------->8-----8<----------(snip snip)
>>This seems to fly in the face of all the "text-book" advice.

>There is no point in initialising variables twice - if you know a variable is
>always assigned a value before it is read there is no point in initialising
>it again at the start of the function.

I was thinking only of auto vars.. So in this...
int func( int a, int b ) {      void * ptr;  /*..A...*/  return ptr;  }
ptr has an indeterminate value at A. Because of this some people advocate
int func( int a, int b ) {      void * ptr = NULL;  /*..A...*/  return ptr;  }

This is generally better because you've nailed it down. I never questioned
it till recently, when I hit the bug mentioned in my the previous posting.

Quote:
>The natural conclusion to that would
>be initialising every variable defined in a function at its definition which
>would be a mess.

Agreed.

Quote:
>I've never come across a text book that suggests this - perhaps you could
>tell us where you saw it and what it actually said?

When I say suggest it, I mean by example. I'm sure we could all find code
in books, or ftp sites that DOES do..
        type var = value;
which is the same as
        type var;
        var = value;

I've decided not to do it anymore on the grounds that

1. My lint can detect a missing assignment in the data flow.
2. There's no hit if the compiler is worth its salt, since even if the code ends up like this...
        void * ptr;
        if (cond) ptr = NULL;
        else if (cond2) ptr = NULL;
        else if (cons3) ptr = NULL;
        else ptr = func();
    there will only be one actual ptr=NULL with internal-gotos
3. The above is easier to read. Code is local and does not
    depend on previous 'state'
4. Used consistently it eliminates all
        type var = value;
    While such a construct is ok-ish in a function { block, what about in a nested
    block, such as a while { inside an if () {  ?
    Even C programmers of a few years have to start thinking,
    "Er does that mean the var is initialised the first time through the while loop
    or every time through the while loop?"
    What I'm trying to say is that I think it makes the function easier to read.

One important point is that many lints will NOT detect the missing assignment.

Cheers
JJ



Mon, 19 May 1997 01:36:23 GMT  
 Variable Initialisation Considered Harmful ?

Quote:


>>It turns out that I can
>>The answer is NOT to initialise the ptr to default value. That way, when you
>> lint it,

>There is no point in initialising variables twice - if you know a variable is

                                                     ^^^^^^^^^^^^^^^^^^^^^^^^
Quote:
>always assigned a value                                                    

 ^^^^^^^^^^^^^^^^^^^^^^^

But you don't always know, particularly if you adding some new function
to a large project written by others. Maybe said pointer will come back
with a garbage value. I'd say it's a better habit to intialize
automatic pointers to NULL, and check the return values before using.
--
/**********************************************************

 **********************************************************/



Mon, 19 May 1997 03:33:04 GMT  
 Variable Initialisation Considered Harmful ?

Quote:

> But you don't always know, particularly if you adding some new function
> to a large project written by others. Maybe said pointer will come back
> with a garbage value. I'd say it's a better habit to intialize
> automatic pointers to NULL, and check the return values before using.

This is a bad practice, because it can hide bugs.  Consider the
following snippet, where searchFunctionIDidntWrite sets *answer to
NULL if the search fails:

void searchFunctionIDidntWrite(void **answer, int lookForMe);

void
myFunction(int param)
{
   void *answer = NULL; /* WARNING: May hide bug. */

   searchFunctionIDidntWrite(&answer, param);
   if(answer != NULL) {
   /* Do stuff with answer */
   }

Quote:
}

If there's a bug in searchFunctionIDidntWrite so that answer never
gets set, you're masking the bug by initializing answer to NULL.  If
answer is left to be garbage, you're likely to trigger a hardware trap*
when you dereference it, and that will throw suspicion on
searchFunctionIDidntWrite.

This one was contrived, but similar situations do occur in the real world.
The best rule is:  Do NOT program defensively if such defensive programming
would hide real bugs.

* Unless you're running under MS-DOG, of course.  But then your problems
are much deeper...

--
David F. Skoll
<a HREF="http://www.doe.carleton.ca/students/dfs/">
Click here for my home page</a> "Query two pi" on typewriter.



Mon, 19 May 1997 06:01:38 GMT  
 Variable Initialisation Considered Harmful ?

Quote:
> The answer is NOT to initialise the ptr to default value. That way, when you lint it,
> if there is a data flow path that does not initialise ptr, lint will warn you.

    So will some compilers. Some will even warn you that the
    initialisation is wasted and has been optimised away if ptr is
    always set to something by the rest of the function.

Quote:
> This seems to fly in the face of all the "text-book" advice.

    What text book advice is this? The reason for initialising the
    pointer is to give it some valid value to return should the body
    of the function not do anything to it, on the assumption that this
    may happen. If the function is reasonably brief, then you could
    probably see at a glance whether it was always being set or not,
    and initialise it or leave it accordingly.

    One related issue is that if the function(s) calling this one
    validated what it returned, as most functions calling others that
    return pointers should probably do, then it would be OK to return
    NULL.

Quote:
> JJ

--

***             Count Templar, ELITE, Cobra Mk III (FM-287)             ***


Mon, 19 May 1997 12:14:11 GMT  
 Variable Initialisation Considered Harmful ?

[...]
: This is a bad practice, because it can hide bugs.  Consider the
: following snippet, where searchFunctionIDidntWrite sets *answer to
: NULL if the search fails:

: void searchFunctionIDidntWrite(void **answer, int lookForMe);

: void
: myFunction(int param)
: {
:    void *answer = NULL; /* WARNING: May hide bug. */

:    searchFunctionIDidntWrite(&answer, param);
:    if(answer != NULL) {
:    /* Do stuff with answer */
:    }
     /* I would add ... */
     else
        {
        Fatal("searchFunctionIDidntWrite() returns NULL");
        /* (or whatever else might be appropriate) */
        }
: }

--
+---------+----------------------------------+--------------------------------+

|    J.   |                                  |  are not necessarily those of  |
|  Brody  | http://www.cloud9.net/~kenbrody/ |  The Small Computer Company."  |
+---------+----------------------------------+--------------------------------+

    UL++++ P+ L+(++) 3- E- N++>+ K(---) W++$ M-- V(-) -po+ Y+ t+ !5 j++ R



Mon, 19 May 1997 22:47:56 GMT  
 Variable Initialisation Considered Harmful ?


Quote:


>>There is no point in initialising variables twice - if you know a variable is
>                                                     ^^^^^^^^^^^^^^^^^^^^^^^^
>>always assigned a value                                                    
> ^^^^^^^^^^^^^^^^^^^^^^^

>But you don't always know, particularly if you adding some new function
>to a large project written by others. Maybe said pointer will come back
>with a garbage value. I'd say it's a better habit to intialize
>automatic pointers to NULL, and check the return values before using.

This is where you need a compiler that tells you when an uninitialised variable
could be accessed. Most decent compilers will (although this check will often
only work when optimisation is enabled).

If you're adding a new function there is no problem since you have
complete control over the variables in that function. There may be a problem
if you are modifying an existing function but this should be rare and if
you don't understand the dataflow in a function you're going to botch it
anyway. Allowing the compiler to warn you about uninitialised variables is
one of the few ways of determining that you've botched it. By initialising
the variable at the start you remove the possibility of this diagnostic.

Testing the return value may have helped in the original example but in
most cases the variable will be used for internal calculation so the
effect on the return value will at best be indirect and may not produce
an obviously bad result.

IMHO the logical conclusion to your argument is that you should *not*
blithely initialise variables.

--
-----------------------------------------


-----------------------------------------



Tue, 20 May 1997 22:08:01 GMT  
 Variable Initialisation Considered Harmful ?

Quote:

>I was thinking only of auto vars.. So in this...
>int func( int a, int b ) {      void * ptr;  /*..A...*/  return ptr;  }
>ptr has an indeterminate value at A. Because of this some people advocate
>int func( int a, int b ) {      void * ptr = NULL;  /*..A...*/  return ptr;  }

>This is generally better because you've nailed it down. I never questioned
>it till recently, when I hit the bug mentioned in my the previous posting.

This is fine as part of the overall design of the function e.g. if the
rest of the function deliberately avoids assigning to ptr in the case of
an error. However is is wrong as a quick-fix to simply avoid potential
uninitialised variable problems.

Quote:
>When I say suggest it, I mean by example. I'm sure we could all find code
>in books, or ftp sites that DOES do..
>        type var = value;
>which is the same as
>        type var;
>        var = value;

If it is necessary for the correct operation of the function then
it is fine. However code like this doesn't imply variable initialisation
for its own sake is a good thing and isn't a recommendation for it.

 .
 .

Quote:
>One important point is that many lints will NOT detect the missing assignment.

I just use gcc which will! :-)

--
-----------------------------------------


-----------------------------------------



Tue, 20 May 1997 22:25:33 GMT  
 Variable Initialisation Considered Harmful ?

Quote:

>I've decided not to do it anymore on the grounds that

>1. My lint can detect a missing assignment in the data flow.

Perhaps when the flow of control is simple as in the case for a series
of if/else-s.  Toss a loop in there, and see what happens.

If you really want to catch this automatically, and be certain
it works in all cases, try this:

        void *foo( void ) {
          void *ptr = LEGAL_BUT_OTHERWISE_UNUSED_VALUE ;

          /* code as usual */

          assert( ptr != LEGAL_BUT_OTHERWISE_UNUSED_VALUE ) ;
          return ptr ;
        }

assert() is really very useful.  In the last year or so, I've started
using it in my code.  I don't know how many hours of debugging it's
saved me, but I can tell you it is much MUCH easier to find a bug
when your program stops because of a failed assertion than it is when
it stops due to some random run-time error.

Every time you write a function, you have certain conditions in mind
that you expect to be met.  The best way to document these assumptions,
as well as make sure they are always true, is to use assert:

        void *bar( glorp *p ) {
          /* this function should never be called with a NULL pointer: */
          assert( p != NULL ) ;
          /* etc */
        }

-Dave
--
***   ***   ***   ***   ***   ***   ***   ***   ***   ***   ***   ***   ***
  *     *     *     *     *     *     *     *     *     *     *     *     *
 *     *     *     *     *     *     *     *     *     *     *     *     *



Mon, 26 May 1997 06:01:34 GMT  
 Variable Initialisation Considered Harmful ?

[program with missing assignement to ptr removed]

Quote:
>The answer is NOT to initialise the ptr to default value. That way, when you lint it,
>if there is a data flow path that does not initialise ptr, lint will warn you.
>This seems to fly in the face of all the "text-book" advice.

I agree with your approach 100%. IF your compiler flags peoperly the
unassigned variables (proper flow analysis) then it is best to remove
the bogus initialization. After all, that initialization is NOT part of
the program logic but a habit some people pick. It producec no benefit
as far as I can tell.

You COULD intialize the whole program to a known state and then track it
with proper asserts, but not a blanket initialization of each variable.

I think that this is historic style from the days when unassigned
variables were not flags or lint was too weak.
--
Regards



Thu, 29 May 1997 18:23:30 GMT  
 
 [ 11 post ] 

 Relevant Pages 

1. Hungarian Notation Can be Considered Harmful (was Re: GCC/G++ vs Other guys)

2. cast considered harmful?

3. break considered harmful

4. strcat() considered harmful

5. Gotos considered harmful (Dijkstra vs. Knuth )

6. Optimization considered harmful

7. Optimization considered harmful

8. EOF considered harmful

9. bitfields considered harmful?

10. C Problem (or, GOTOs considered harmful)

11. Mixed prototyping considered harmful

12. GOTO considered harmful

 

 
Powered by phpBB® Forum Software