[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