Patch from 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.

git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1270 9efc2488-bf62-4759-914b-345cdb29e865

diff --git a/include/opensrf/osrf_json.h b/include/opensrf/osrf_json.h
index 5312fec..4fadfda 100644 (file)
--- a/include/opensrf/osrf_json.h
+++ b/include/opensrf/osrf_json.h
@@ -13,15 +13,13 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU General Public License for more details.
 */
 
+#ifndef _JSON_H
+#define _JSON_H
 
 #include <opensrf/utils.h>
 #include <opensrf/osrf_list.h>
 #include <opensrf/osrf_hash.h>
 
-#ifndef _JSON_H
-#define _JSON_H
-
-
 /* parser states */
 #define JSON_STATE_IN_OBJECT   0x1
 #define JSON_STATE_IN_ARRAY            0x2
@@ -352,7 +350,7 @@ char* jsonScrubNumber( const char* s );
        Note also that the object returned is a clone and
        must be freed by the caller
 */
-jsonObject* jsonObjectFindPath( const jsonObject* obj, char* path, ... );
+jsonObject* jsonObjectFindPath( const jsonObject* obj, const char* path, ... );
 
 
 /* formats a JSON string from printing.  User must free returned string */

diff --git a/include/opensrf/osrf_json_utils.h b/include/opensrf/osrf_json_utils.h
index 3ab75a0..53faefa 100644 (file)
--- a/include/opensrf/osrf_json_utils.h
+++ b/include/opensrf/osrf_json_utils.h
@@ -13,6 +13,8 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU General Public License for more details.
 */
 
+#ifndef OSRF_JSON_UTILS_H
+#define OSRF_JSON_UTILS_H
 
 /* ----------------------------------------------------------------------- */
 /* Clients need not include this file.  These are internal utilities only      */
@@ -94,14 +96,5 @@ int _jsonParserHandleNumber( jsonParserContext* ctx );
 
 void _jsonInsertParserItem( jsonInternalParser* p, jsonObject* newo );
 
-
-/* Utility method. finds any object in the tree that matches the path.  
-       Use this for finding paths that start with '//' */
-jsonObject* _jsonObjectFindPathRecurse( const jsonObject* o, char* root, char* path );
-
-
-/* returns a list of object whose key is 'root'.  These are used as
-       potential objects when doing a // search */
-jsonObject* __jsonObjectFindPathRecurse( const jsonObject* o, char* root );
-
+#endif
 

diff --git a/src/libopensrf/osrf_json_tools.c b/src/libopensrf/osrf_json_tools.c
index 76b5c41..0d33acf 100644 (file)
--- a/src/libopensrf/osrf_json_tools.c
+++ b/src/libopensrf/osrf_json_tools.c
@@ -16,6 +16,9 @@ GNU General Public License for more details.
 #include <opensrf/osrf_json.h>
 #include <opensrf/osrf_json_utils.h>
 
+static jsonObject* findMultiPath( const jsonObject* o,
+               const char* root, const char* path );
+static jsonObject* findMultiPathRecurse( const jsonObject* o, const char* root );
 static jsonObject* _jsonObjectEncodeClass( const jsonObject* obj, int ignoreClass );
 
 static char* _tabs(int count) {
@@ -178,53 +181,56 @@ jsonObject* jsonParseFile( const char* filename ) {
 
 
 
-jsonObject* jsonObjectFindPath( const jsonObject* obj, char* format, ...) {
+jsonObject* jsonObjectFindPath( const jsonObject* obj, const char* format, ...) {
        if(!obj || !format || strlen(format) < 1) return NULL;  
 
        VA_LIST_TO_STRING(format);
        char* buf = VA_BUF;
        char* token = NULL;
-       char* t = buf;
        char* tt; /* strtok storage */
 
-       /* copy the path before strtok_r destroys it */
-       char* pathcopy = strdup(buf);
+       /* special case where path starts with //  (start anywhere) */
+       if(buf[0] == '/' && buf[1] == '/' && buf[2] != '\0') {
 
-       /* grab the root of the path */
-       token = strtok_r(t, "/", &tt);
-       if(!token) return NULL;
+               /* copy the path before strtok_r destroys it */
+               char* pathcopy = strdup(buf);
 
-       /* special case where path starts with //  (start anywhere) */
-       if(strlen(pathcopy) > 2 && pathcopy[0] == '/' && pathcopy[1] == '/') {
-               jsonObject* it = _jsonObjectFindPathRecurse(obj, token, pathcopy + 1);
+               /* grab the root of the path */
+               token = strtok_r(buf, "/", &tt);
+               if(!token) {
+                       free(pathcopy);
+                       return NULL;
+               }
+
+               jsonObject* it = findMultiPath(obj, token, pathcopy + 1);
                free(pathcopy);
                return it;
        }
+       else
+       {
+               /* grab the root of the path */
+               token = strtok_r(buf, "/", &tt);
+               if(!token) return NULL;
 
-       free(pathcopy);
+               do {
+                       obj = jsonObjectGetKeyConst(obj, token);
+               } while( (token = strtok_r(NULL, "/", &tt)) && obj);
 
-       t = NULL;
-       do { 
-               obj = jsonObjectGetKeyConst(obj, token);
-       } while( (token = strtok_r(NULL, "/", &tt)) && obj);
-
-       return jsonObjectClone(obj);
+               return jsonObjectClone(obj);
+       }
 }
 
 /* --------------------------------------------------------------- */
 
-
-
-jsonObject* _jsonObjectFindPathRecurse(const jsonObject* obj, char* root, char* path) {
+/* Utility method. finds any object in the tree that matches the path.  
+       Use this for finding paths that start with '//' */
+static jsonObject* findMultiPath(const jsonObject* obj,
+               const char* root, const char* path) {
 
        if(!obj || ! root || !path) return NULL;
 
        /* collect all of the potential objects */
-       jsonObject* arr = __jsonObjectFindPathRecurse(obj, root);
-
-       /* container for fully matching objects */
-       jsonObject* newarr = jsonParseString("[]");
-       int i;
+       jsonObject* arr = findMultiPathRecurse(obj, root);
 
        /* path is just /root or /root/ */
        if( strlen(root) + 2 >= strlen(path) ) {
@@ -232,32 +238,37 @@ jsonObject* _jsonObjectFindPathRecurse(const jsonObject* obj, char* root, char*
 
        } else {
 
+               /* container for fully matching objects */
+               jsonObject* newarr = jsonNewObjectType(JSON_ARRAY);
+               int i;
+
                /* gather all of the sub-objects that match the full path */
                for( i = 0; i < arr->size; i++ ) {
                        const jsonObject* a = jsonObjectGetIndex(arr, i);
                        jsonObject* thing = jsonObjectFindPath(a , path + strlen(root) + 1); 
 
                        if(thing) { //jsonObjectPush(newarr, thing);
-               if(thing->type == JSON_ARRAY) {
-               int i;
+                               if(thing->type == JSON_ARRAY) {
+                       int i;
                                        for( i = 0; i != thing->size; i++ )
                                                jsonObjectPush(newarr, jsonObjectClone(jsonObjectGetIndex(thing,i)));
                                        jsonObjectFree(thing);
-
                                } else {
                                        jsonObjectPush(newarr, thing);
-                               }                                               
+                               }
                        }
                }
+
+               jsonObjectFree(arr);
+               return newarr;
        }
-       
-       jsonObjectFree(arr);
-       return newarr;
 }
 
-jsonObject* __jsonObjectFindPathRecurse(const jsonObject* obj, char* root) {
+/* returns a list of object whose key is 'root'.  These are used as
+       potential objects when doing a // search */
+static jsonObject* findMultiPathRecurse(const jsonObject* obj, const char* root) {
 
-       jsonObject* arr = jsonParseString("[]");
+       jsonObject* arr = jsonNewObjectType(JSON_ARRAY);
        if(!obj) return arr;
 
        int i;
@@ -273,7 +284,7 @@ jsonObject* __jsonObjectFindPathRecurse(const jsonObject* obj, char* root) {
 
        /* recurse through the children and find all potential nodes */
        while( (tmp = jsonIteratorNext(itr)) ) {
-               childarr = __jsonObjectFindPathRecurse(tmp, root);
+               childarr = findMultiPathRecurse(tmp, root);
                if(childarr && childarr->size > 0) {
                        for( i = 0; i!= childarr->size; i++ ) {
                                jsonObjectPush( arr, jsonObjectClone(jsonObjectGetIndex(childarr, i)) );