[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