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

Scott McKellar mck9 at swbell.net
Wed Dec 3 19:22:07 EST 2008


These patches do various things:

1. Move the declaration of osrf_app_request_struct, and its typedef as
osrfAppRequest, out of the header and into osrf_app_session.c.

2. In the declaration of osrf_app_session_struct: remove an obsolete 
and commented-out declaration of request_queue.

3. Abolished _osrf_app_session_free(), moving its contents into 
osrfAppSessionFree().

4. In osrfAppSessionCleanup(): after freeing the cache, nullify the
pointer to it, in the interests of good hygiene.

5. In _osrf_app_request_free(): free the messages in the result queue.

6. In _osrf_app_request_recv(): Eliminated the useless intermediate
variables tmp_msg used for dequeuing messages.

7. In osrf_app_session_set_locale: If the existing session_locale is
big enough to hold the new locale, use strcpy() instead of free() and
strdup().

8. In osrf_app_session_set_remote: If the existing remote_id is big
enough to hold the new remote_id, use strcpy() instead of free() and
strdup().

9. To eliminate some duplication of code, call 
osrf_app_session_set_locale() amd osrf_app_session_set_remote() in
_osrf_app_request_recv() and osrf_app_session_reset_remote(),
respectively.

10. Performance tweak: in osrfAppRequestRespondComplete: don't create
the payload message unless we're actually going to use it.

11. Make osrfAppSessionCache static, since no other source file
references it.

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

Discussion (using the same numbering as above):

1: osrfAppRequest is an implementation detail of osrfAppSession.  No
other source file knows anything about it, or needs to.

2: Originally, request_queue was probably a linked list.  Now it's an
osrfHash, and has been for some time.  By now, the old obsolete
declaration contributes nothing but clutter.

3: These two functions are effectively inseparable.  Each does stuff
that should never be done without doing the other stuff too.  We might
as well marry them.

5. This change plugs a potential memory leak.  Maybe there's a good
reason why we don't free the messages in the result queue, but I
don't see it.  Normally the queue is probably empty by the time we
destroy the osrfAppRequest, but it doesn't cost much to check.

7 and 8: These are performance tweaks, to avoid the churning of memory.
I suspect that most locales are the same size, and most remote_ids are
the same size, so we should be shaving off some CPU cycles most of the
time.  The main cost is a couple of calls to strlen(), which should be
cheap (especially for locales, which are generally pretty short).

9: These changes not only eliminate duplication but also take advantage
of the performance tweaks in items 7 and 8.

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: osrf_app_session_h_6.patch
Type: text/x-patch
Size: 1285 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081203/45e147d1/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_app_session_c_9.patch
Type: text/x-patch
Size: 10985 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081203/45e147d1/attachment-0001.bin 


More information about the Open-ils-dev mailing list