[OPEN-ILS-DEV] C nits: object.c (Part 3)

Scott McKellar mck9 at swbell.net
Sat Apr 7 13:17:03 EDT 2007


In many places, object.c invokes undefined behavior, due to an unstated
but pervasive assumption that NULL is represented by all-bits-zero.

For example, in jsonObjectNodeFree() we see the line:

	free(node->key);

Unless node->key contains either NULL or a pointer returned from
malloc(), the results are undefined.

When we create a jsonObjectNode in jsonObjectNode(), we allocate the
memory with safe_malloc(), which fills the memory with binary zeros.
We do not otherwise initialize the key member.

In systems that represent NULL as all-bits-zero, the effect is the 
same as assigning NULL to this member.  However the C Standard does
not require this representation of NULL.  In a system that represents
NULL differently (and some do), all-bits-zero is simply an invalid 
pointer.  As a result, the code in object.c will break in innumerable 
ways, of which the above is only one example.

The solution is simple.  Whenever we allocate and initialize a
structure, we should explicitly initialize every pointer member to 
NULL, or to some other appropriate value (where all-bits-zero is not 
an appropriate value).

In jsonNewObjectNode() we should set node->key to NULL.

In jsonNewObject() we should set obj->classname and obj->comment to
NULL.  It's not so clear what to do with obj->value, because it is
a union of a pointer with several non pointer types.  I recommend 
setting c, the pointer member of the union, to NULL, and letting the
non-pointer members fend for themselves.

Scott McKellar
http://home.swbell.net/mck9/aargh/



More information about the Open-ils-dev mailing list