[OPEN-ILS-DEV] PATCH: oils_requestor.c possible bug

[Scott McKellar]

This patch is speculative.  I think it fixes a bug, but I can't be
completely sure because I don't really know how the program is 
supposed to behave.

The program reads one or more lines from standard input.  Unless the
line is "quit" or "exit", we use strtok_r() to break up the line
into a series of space-separated tokens.  The first two tokens are
treated as names for a service and a method.  Any remaining tokens
are concatenated into a single string, with no intervening spaces.
The resulting string is then enclosed in square brackets to be parsed
as JSON text.

What bothers me is that we remove all blank spaces from the JSON text.
For most purposes it doesn't matter, because the JSON parser would
skip over any blank spaces between tokens anyway.  However what
the parser considers a token and what strtok_r() considers a token
are two different things.  In particular the parser considers a
string literal to be a token, but if it contains embedded blanks,
strtok_r() considers it to be two or more tokens.

As a result we eliminate any spaces embedded within string literals 
before the parser gets to see them.  "The Old Man and the Sea"
becomes "TheOldManandtheSea".

I could be mistaken, but I have trouble believing that this behavior
was intended.  I think it's a bug, and here's the fix.

I only call strtok_r() twice, once to get the service and once to
get the method.  The rest of the line I pass intact to the JSON
parser, sandwiched between square brackets.

The new code should also incur less overhead.  I avoid at least one
call to strtok_r().  I call buffer_add_char() and buffer_add() instead
of the more expensive buffer_fadd(), and only when there is something
beyond the service and the method.  I call jsonParseString() instead
of the more expensive jsonParseStringFmt().  I create a growing_buffer
only when I have something to put into it.

In order to test these changes I wrote stubs for some of the 
underlying functions such as osrf_app_client_session_init() and
osrf_app_session_make_req().  Nothing is commented out in 
oils_requestor.c itself.  The patch is based on the version currently
in the CVS repository.  I don't believe there will be any interaction 
between this patch and the one I submitted last week.

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: oils_requestor_c_2.patch
Type: text/x-patch
Size: 1808 bytes
Desc: 2393042016-oils_requestor_c_2.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20070507/ca1a35ca/oils_requestor_c_2.bin