[OPEN-ILS-DEV] PATCH: jsonObjectFindPath

[Scott McKellar]

These patches mostly concern the jsonObjectFindPath function in
osrf_json_tools.c, along with a couple of related functions.  They
plug some memory leaks and boost performance.

1. In osrf_json.h: I moved the nested #includes inside the compilation
guard.

2. In osrf_json_utils.h: I added a compilation guard.

3. I moved the declarations of _jsonObjectF_jsonObjectFindPathRecurse 
and __jsonObjectF_jsonObjectFindPathRecurse out of the header and into
the implementation file, and made them both static.

4. I also renamed those functions to findMultiPath and 
findMultiPathRecurse, respectively, so that their names wouldn't be
confusingly similar.

5. In both functions, and in the parent function jsonObjectFindPath(),
I added the const qualifier to the character pointer parameters.

6. In jsonObjectFindPath(): we were leaking pathcopy in the case of
an early return.  Besides plugging that leak, I rearranged the logic
so that we don't allocate pathcopy unless we actually have a use for
it.

7. Also in jsonObjectFindPath(): I eliminated a call to strlen(), and
the redundant variable t.

8. In _jsonObjectFindPathRecurse() (now findMultiPath()) we were
leaking newarr in the case of an early return.  Besides plugging that
leak, I rearranged the logic so that we don't allocate newarr unless
we actually have a use for it.

9. In a couple of spots I replaced "jsonParseString("[]")" with
"jsonNewObjectType(JSON_ARRAY)", a call that produces the same result
with a lot less overhead.

---------

The use of strtok_r() in these functions creates a subtle potential
bug that doesn't apply to Evergreen but may apply to some other
arbitrary application.

In JSON, the name string of a name/value pair may contain any
character, though some characters must be escaped by preceding
backslashes.  In particular, a name may contain a forward slash.

The path functions will see such an embedded slash as a separator
between path nodes.  As a result they will fail to find the intended
path.  Granted, it would be perverse to include a slash in a name and
then try to find a path with it, but it is conceivable.

The solution -- if a solution is needed -- is to define some
convention to distinguish between embedded slashes and separator
slashes.  For example, we could decree that an embedded slash be
escaped by a preceding backslash.  However neither strtok nor its
safer sister strtok_r is smart enough to recognize that distinction.

Wnenever I have been tempted to use strtok(), I have invariably
backed off, because of some bug or other that would result.  The
main problem lies with the use of delimiter characters to separate
fields.  In most cases there are circumstances where the delimiter
character may be embedded in the data.  Various conventions may be
used to distinguish these occurrences -- doubling, backlashes, quote
marks, etc. -- but strtok() won't recognize them.

As a result I have never used strtok() or strtok_r() in my own code.
When I see them in somebody else's code, I look for a bug.  I usually
find one.

(Whoever wrote the man page for strtok on my system evidently agrees
with me, though possibly for different reasons.  The section on BUGS
starts out: "Never use these functions.")

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_json_h_11.patch
Type: text/x-patch
Size: 1328 bytes
Desc: 3787656320-osrf_json_h_11.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20080217/dae3d256/osrf_json_h_11.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_json_utils_h_4.patch
Type: text/x-patch
Size: 1133 bytes
Desc: 3011786863-osrf_json_utils_h_4.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20080217/dae3d256/osrf_json_utils_h_4.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_json_tools_c_5.patch
Type: text/x-patch
Size: 6298 bytes
Desc: 3546604339-osrf_json_tools_c_5.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20080217/dae3d256/osrf_json_tools_c_5.bin