From: miker Date: Mon, 10 Mar 2008 02:54:12 +0000 (+0000) Subject: Patch from Scott McKellar: X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=b3c7a1c55ee0210bb3b076480b38d723e730462d;p=working%2FOpenSRF.git 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 --- 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 #include #include -#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 --- 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 --- 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 #include +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)) );