[OPEN-ILS-DEV] PATCH: transport_message.[ch] (miscellaneous)

Scott McKellar mck9 at swbell.net
Tue Dec 25 01:13:36 EST 2007


This patch is a miscellaneous cleanup.  Summary:

1. Added the const qualifier in various places.

2. Eliminated some unnecessary calls to strlen(), where they were
used merely to determine whether a string was empty.

3. Ensured that all members of a new transport_message are 
explicitly populated.

4. Plugged a memory leak in the case where strdup() fails.

5. Eliminated some unhelpful casts of malloc'd pointers.

6. Added some protective tests for NULL pointer parameters.

7. In several spots I replaced numeric literals with character 
literals, both to make the code more readable and to avoid a needless
dependence on ASCII.

8. Rewrote the jid_get_resource function to avoid repeatedly
overwriting the same buffer.

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

Commentary:

1. After using strdup() to populate several members, message_init()
checks to see if any of the strdups failed.  If so, it issues an 
error message and returns NULL, but without freeing anything that it
had allocated.  I added code to free the newly allocated
transport_message and any successfully strdup'd strings.

I don't know why we bother to check strdup() here.  The rest of the
code base is littered with strdups, and we almost never check for
NULL returns.  If we are really concerned that strdup() might fail,
we should create a safe_strdup() analogous to safe_malloc().

If we confine ourselves to Linux, safe_* functions are probably a
waste of time anyway.  I have read in more than one place that, in
Linux, malloc() NEVER returns NULL.  It always returns a non-NULL
pointer, without checking to see that physical memory is actually
available to satisfy the request.  When you really do run out of
memory, the allocation seems to succeed, but bad things happen when
you try to access the memory.  As a result, you can't protect 
yourself by checking for a NULL return from malloc() (nor by any 
other means).

2. The jid_get_resource function examines an input buffer and copies
everything past the last forward slash ('/'), up to a maximum size,
to an output buffer.

The original version of this function made such a copy for each
slash it found.  If the input string contained three slashes, the
function would copy three different strings to the output buffer,
and only the last one would survive.

My version uses strrchr() to find the last slash, and then writes 
to the output buffer just once.

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

Finally: a very merry Dies Natalis Solis Invicti to all.

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

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: transport_message_h_1.patch
Type: text/x-patch
Size: 2567 bytes
Desc: 588098499-transport_message_h_1.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071224/befe0f74/transport_message_h_1.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: transport_message_c_1.patch
Type: text/x-patch
Size: 15327 bytes
Desc: 1827854496-transport_message_c_1.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071224/befe0f74/transport_message_c_1.bin


More information about the Open-ils-dev mailing list