[OPEN-ILS-DEV] PATCH: utils.c (miscellaneous)

[Scott McKellar]

This patch is mostly a couple of tweaks to the growing_buffer code, loosely
related to my previous patch to utils.h.  There is also a small tweak to
uescape().

1. in buffer_add() I replaced strcat() with strcpy() for appending the new
string.  Since we already know where the end of the old string is, we don't
need to ask strcat() to find it for us.

2. In buffer_reset(), the old code contains the following:

    osrf_clearbuf( gb->buf, sizeof(gb->buf) );

The evident intent is to clear the buffer.  However sizeof(gb->buf) is not
the size of the buffer, it's the size of the pointer to the buffer.  We
were clearing only the first four bytes or so.  I changed the line to:

    osrf_clearbuf( gb->buf, gb->size );

3. Also in buffer_reset(), I added a line to populate the first byte of
the buffer with a nul, to ensure that the length of the (empty) string matches the n_used member.

4. In uescape(), we were examining the contents of string[] without first
verifying that string was not NULL.  The result would be undefined
behavior if string were ever NULL.  I added a couple of lines to treat
a NULL pointer as if it were a pointer to an empty string.

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

Together with yesterday's patch to utils.h, these changes are intended
to guarantee that the length of the string stored in a growing_buffer
always matches the value of n_used.  buffer_data() relies on such a
match.  So did buffer_add() (and, indirectly, buffer_fadd()) until I 
changed it.  So does every bit of code that calls buffer_release().

Previously the terminal nuls were supplied by filling the entire buffer
with nuls before otherwise populating it.  This practice always bothered
me because it looked like a waste of CPU cycles.

The use of osrf_clearbuf() in buffer_reset() is probably no longer
necessary or useful.  However, from an abundance of caution I have left it
in there.  I cannot guarantee that there is no other bit of code that fails
to append a terminal nul where it should.  It might be a useful experiment
to eliminate the prenullification and see if anything breaks.

Another possible experiment is to insert the assert() macro at the entry
and exit of each of the growing_buffer functions:

    assert( strlen( gb->buf ) == gb->n_used );

Such an assertion would add overhead for development and debugging, but
would reduce to a no-op when NDEBUG is #defined.

Scott McKellar
http://home.swbell.net/mck9/ct/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: utils_c_9.patch
Type: text/x-patch
Size: 1154 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081114/8e49f501/attachment.bin