[OPEN-ILS-DEV] PATCH: transport_client.[ch] (message queue)

[Scott McKellar]

These patches restructure the mechanism for queueing incoming transport
messages.  In addition, the patch to transport_client.c rearranges the
logic a bit in client_recv().

1. A transport_message now carries a pointer to be used in a linked list.
It is initialized to NULL when the message is created.  We no longer use
a separately allocated list node to carry the message.

2. The queue of transport_messages no longer starts with a dummy node.

3. Instead of finding the tail of the queue by traversing the list from
the head, we maintain a separate pointer to the tail node.  Thus the
enqueuing operation occurs in constant time instead of linear time.

4. In client_recv: we now have the dequeueing code in a single place,
instead of duplicating it.

5. In client_recv: I eliminated some conditional compilation that made
no real difference, since both branches of the #ifdef were effectively
identical.

6. In client_recv: changed both loops from while loops to do-while
loops, since in each case we want to perform at least one iteration.

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

The changes to the message queue follow what I outlined in a previous
post.

I understand Mike's discomfort with conflating the container with the
contained, but I have three counter-argruments.  First, it is inherent
in messages that they be queued.  The message itself might as well 
provide a suitable hook.  Second, the transport_message, transport_client,
and transport_session are all so tightly coupled to each other anyway 
that there's not much point in maintaining a pretense of independence.
Third, we have the venerable precedent of osrfMessage, which carries a
similar linkage pointer for the convenience of other levels of code.

The other changes to client_recv() were a case of one thing leading to
another.

First, it annoyed me to have the dequeueing logic in two different 
places.  I rearranged the logic so that we don't dequeue anything until
we have made every effort to make sure that the queue isn't empty.
Then we can dequeue in just one spot.

(An old classmate of mine once remarked that "queue" is the only word
in the English language that is 80% superfluous.)

Next I eliminated the conditional compilation, which in this case didn't
accomplish anything, and which probably wouldn't work very well anyway
if you use shared libraries for this stuff.

What was wrapped in the #ifdef was a break statement, subject to an if,
and it looked a little awkward.  I think the intent was to  make sure
that we perform at least one iteration even if the timeout is zero.  A
more graceful way to do that is to use a do-while loop instead of a
while loop, so that's what I did.

If the timeout parameter is zero, the results will be the same. Otherwise
there will be a subtle difference.  With the old logic, even when the
time remaining reaches zero, we'll still make one last attempt to read
a message.  With the new logic we won't.  My sense of the matter is that
this difference is too slight to worry about, especially since the same
timeout is applied by the underlying select() call.  If it matters, we
can go back to the old logic with a break statement.

Then I looked at the other loop, where timeout = -1, and changed it to
a do-while as well.  Since we have already determined that the queue is
empty, we don't need to check the queue again at the top of the loop.
More importantly, the code is easier to read if both loops have almost
the same structure.

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_2.patch
Type: text/x-patch
Size: 446 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081207/bcbd6c18/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: transport_message_c_3.patch
Type: text/x-patch
Size: 1007 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081207/bcbd6c18/attachment-0001.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: transport_client_h_3.patch
Type: text/x-patch
Size: 815 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081207/bcbd6c18/attachment-0002.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: transport_client_c_4.patch
Type: text/x-patch
Size: 8252 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081207/bcbd6c18/attachment-0003.bin