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

[Scott McKellar]

>From looking at the code in object.c so far, I have drawn the 
following inferences:

1. When a jsonObject is of type JSON_ARRAY, it uses a singly-linked
list of jsonObjectNodes to simulate a (potentially) sparse array.

2. Each node carries a zero-based index number signifying its logical 
position within the simulated array.

3. Within the list, the nodes are sorted by index number in ascending
order.

4. The index numbers are not necessarily contiguous, i.e. there may
be gaps.  Nodes that are adjacent in the list need not have adjacent
index numbers.

The rest of this discussion is based on the above assumptions.  If
my assumptions are wrong, then my further conclusions may be wrong 
as well.

Consider the function jsonObjectPush().  It appends a node to the 
list and assigns it a corresponding index number.  For example, if 
the list has three nodes in the list, and we add a fourth, we add
it to the end of the list, and give it an index number of 3.

What concerns me is the possibility that we use this function on a
list that has gaps in it.

Suppose, for example, that the list initially has three nodes, with 
index numbers 2, 7, and 9.  After calling jsonObjectPush() to add a 
fourth node, the four nodes will be out of sequence: 2, 7, 9, 3.

Likewise if the initial sequence is 3, 7, 9. not only will the
resulting list be out of sequence, but it will also contain two
nodes with the same index number of 3.

Assuming that these results are not desirable, the way to fix this
function is to assign an index number that is one greater than that
of the last node.  For example (warning: uncompiled and untested):

unsigned long jsonObjectPush( jsonObject* obj, jsonObject* new_obj) {
	if(!obj) return -1;
	
	obj->type = JSON_ARRAY;

	if(new_obj == NULL) {
		new_obj = jsonNewObject(NULL);
		new_obj->type = JSON_NULL;
	}

	jsonObjectNode* node = jsonNewObjectNode(new_obj);

	if(obj->value.c == NULL) {
		node-index = 0;
		obj->value.c = node;

	} else {
		/* append the node onto the end */
		jsonObjectNode* tmp = obj->value.c;
		while(tmp) {
			if(tmp->next == NULL) break;
			tmp = tmp->next;
		}
		node->index = tmp->index + 1;
		tmp->next = node;
	}
	return ++obj->size;
}

In practice, it may well be that we never call jsonObjectPush() for
a list where the incex numbers are not contiguous.  If that's the
case, then the my proposed changes are not necessary.  However my
proposed version will work whether the index numbers in the existing
list are contiguous are not.  There is some virtue in robustness.

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