> 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.
Also applied. > > -------------- > > 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. I bet there are still some dragons lurking, so your caution is appreciated. Thanks again, -b -- Bill Erickson | VP, Software Development & Integration | Equinox Software, Inc. / The Evergreen Experts | phone: 877-OPEN-ILS (673-6457) | email: [EMAIL PROTECTED] | web: http://esilibrary.com