[OPEN-ILS-DEV] PATCH: string_array.c

[Scott McKellar]

The attached patches tidy up a variety of things in string_array.c
and the associated header.  In at least some small ways the new
version will behave differently.  I believe that these changes are
benign, but you shouldn't take my word for it.

The third attachment is the test driver that I used to verify
that the modified functions work as intended.


1. I added the const qualifier to a number of function parameters.
That's why I'm patching the header as well as the implementation.

2. I added summary comments for some of the functions.

3. init_string_array() takes an int parameter specifying the initial
number of strings to be stored.  If this parameter is zero or
negative, the code won't behave very gracefully.  I added a couple
of lines to apply a plausible default in this case, so that the
function will work usefully even in the face of nonsensical input.
I arbitrarily chose 4 for the default value.  Some other default
might be better, depending on how string arrays are typically used.

4. I tightened up various sanity checks, mostly to verify that the
array member is not NULL.

5. I tried to eliminate the presumption that the pointer array had
been initialized to NULLs by setting all bits to zero (since not
all implementations represent NULL as all-bits-zero).  I considered
initializing each pointer explicitly to NULL, but that seemed like
overkill.  It is probably enough to set the first unused pointer
to NULL.  That's what I did.

6. In a couple of spots I eliminated the use of the
osrfStringArrayGetString function, in favor of accessing the array
directly by subscript.  So far as I can tell, the only reason to use
this function is to apply sanity checks, especially a range check on
the subscript.  However in the contexts where this function was
being used, it could be shown that the sanity checks were unneeded
because everything was sane, including the subscript.

7. In osrfStringArrayRemove(), I eliminated the access of the [n+1]
member (as I discussed in a previous post) by setting array[n] 
explicitly to NULL.  This change avoids the assumption that NULL is
all-bits-zero.  It also avoids the implicit and non-obvious reliance
on the fact that string_array_add() never lets the buffer become
completely full.  The new version of osrfStringArrayRemove() will
work correctly even if the array is full.

8. Also in osrfStringArrayRemove(): I changed the behavior of the
code when the target string is not present in the array.  In such
a case, the old code unconditionally decrements the size member, 
effectively deleting the last string, though without deallocating
it.  If the array is empty, the size member becomes negative --
probably not a good thing.  I doubt that this behavior was intended.
The new version decrements the size member only when the target
string is found within the array.

9. In init_string_array() I considered initializing lhe
total_string_size member explicitly to zero, rather than rely on
the implicit initialization by memset().  However I decided to
leave it alone when I noticed that nothing in string_array.c makes
any reference to this member.  In fact I grepped the whole codebase
for this member and got no hits outside of the header file.  We
can worry about initializing this member if and when it is ever
used for something.

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

Developer’s Certificate of Origin 1.1 By making a contribution to this
project, I certify that:

(a) The contribution was created in whole or in part by me and I have
the right to submit it under the open source license indicated in the
file; or

(b) The contribution is based upon previous work that, to the best of
my knowledge, is covered under an appropriate open source license and I
have the right under that license to submit that work with
modifications, whether created in whole or in part by me, under the
same open source license (unless I am permitted to submit under a
different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person
who certified (a), (b) or (c) and I have not modified it; and

(d) In the case of each of (a), (b), or (c), I understand and agree
that this project and the contribution are public and that a record of
the contribution (including all personal information I submit with it,
including my sign-off) is maintained indefinitely and may be
redistributed consistent with this project or the open source license
indicated in the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: string_array_c_1.patch
Type: text/x-patch
Size: 4766 bytes
Desc: 171816963-string_array_c_1.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070428/4892eb5c/string_array_c_1.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: string_array_h_1.patch
Type: text/x-patch
Size: 1714 bytes
Desc: 3005700809-string_array_h_1.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070428/4892eb5c/string_array_h_1.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sarray.c
Type: text/x-csrc
Size: 3510 bytes
Desc: 3843790055-sarray.c
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070428/4892eb5c/sarray.bin