Patch from Scott McKellar:
authormiker <miker@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Sun, 3 Feb 2008 19:08:44 +0000 (19:08 +0000)
committermiker <miker@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Sun, 3 Feb 2008 19:08:44 +0000 (19:08 +0000)
1. In setSavepoint(), releaseSavepoint() and rollbackSavepoint()
we were leaking spName.

2. Deep in doCreate() we were passing the return value of
jsonObjectToSimpleString() directly to strcmp(), resulting in a leak.
The strcmp() was inside a complex if condition. which I rearranged so
as to capture the string and free it.

Also: I captured and reused the return value from
jsonObjectGetKeyConst() so as to avoid duplicated calls.

Aso: I reversed the sense of the if condition and swapped the branches,
so that it tests for equality rather than inequality.  To my eyes this
arrangement is more readable.

3. doRetrieve() was leaking id.

4. jsonNumberToDBString() was passing the return value of
jsonObjectToSimpleString() directly to atol() and atof(), thereby
leaking the memory.  I captured the pointers and freed them.

5. searchFieldTransform() was leaking val.

6. In searchJOIN() we were leaking type and filter_op in the case of
some early returns.  I moved the allocations past the early returns
so that we don't allocate them until we need them.  I also free them
as soon as we are done with them.  As a side benefit, I was able to
avoid allocating filter_op at all in some cases.

I gave similar treatment to table, although that wasn't being leaked.
As a result I could avoid having to free it in the early returns.

A couple of the early returns would leak field or fkey.  I plugged
those leaks as well.

I moved the declarations of filter and join_filter to their points
of first use, in the interest of clarity.

7. In buildSELECT(): we were passing the return value of
jsonObjectToSimpleString() directly to osrfHashGet(), thereby leaking
the memory.  I captured the pointer and freed it.

8. In doFieldmapperSearch() a do/while loop allocates pkey_val but
in some cases wasn't freeing it.

git-svn-id: svn://svn.open-ils.org/ILS/trunk@8589 dcc99617-32d9-48b4-a31d-7c20da2025e4

Open-ILS/src/c-apps/oils_cstore.c

index b7d6e8a..fb1ea2b 100644 (file)
@@ -463,8 +463,6 @@ int beginTransaction ( osrfMethodContext* ctx ) {
 int setSavepoint ( osrfMethodContext* ctx ) {
        OSRF_METHOD_VERIFY_CONTEXT(ctx);
 
-       char* spName = jsonObjectToSimpleString(jsonObjectGetIndex(ctx->params, 0));
-
        if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
                osrfAppSessionStatus(
                        ctx->session,
@@ -476,6 +474,8 @@ int setSavepoint ( osrfMethodContext* ctx ) {
                return -1;
        }
 
+       char* spName = jsonObjectToSimpleString(jsonObjectGetIndex(ctx->params, 0));
+
        dbi_result result = dbi_conn_queryf(writehandle, "SAVEPOINT \"%s\";", spName);
        if (!result) {
                osrfLogError(
@@ -486,20 +486,20 @@ int setSavepoint ( osrfMethodContext* ctx ) {
                        osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )
                );
                osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, "osrfMethodException", ctx->request, "Error creating savepoint" );
+               free(spName);
                return -1;
        } else {
                jsonObject* ret = jsonNewObject(spName);
                osrfAppRespondComplete( ctx, ret );
                jsonObjectFree(ret);
        }
+       free(spName);
        return 0;
 }
 
 int releaseSavepoint ( osrfMethodContext* ctx ) {
        OSRF_METHOD_VERIFY_CONTEXT(ctx);
 
-       char* spName = jsonObjectToSimpleString(jsonObjectGetIndex(ctx->params, 0));
-
        if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
                osrfAppSessionStatus(
                        ctx->session,
@@ -511,6 +511,8 @@ int releaseSavepoint ( osrfMethodContext* ctx ) {
                return -1;
        }
 
+       char* spName = jsonObjectToSimpleString(jsonObjectGetIndex(ctx->params, 0));
+
        dbi_result result = dbi_conn_queryf(writehandle, "RELEASE SAVEPOINT \"%s\";", spName);
        if (!result) {
                osrfLogError(
@@ -521,20 +523,20 @@ int releaseSavepoint ( osrfMethodContext* ctx ) {
                        osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )
                );
                osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, "osrfMethodException", ctx->request, "Error releasing savepoint" );
+               free(spName);
                return -1;
        } else {
                jsonObject* ret = jsonNewObject(spName);
                osrfAppRespondComplete( ctx, ret );
                jsonObjectFree(ret);
        }
+       free(spName);
        return 0;
 }
 
 int rollbackSavepoint ( osrfMethodContext* ctx ) {
        OSRF_METHOD_VERIFY_CONTEXT(ctx);
 
-       char* spName = jsonObjectToSimpleString(jsonObjectGetIndex(ctx->params, 0));
-
        if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
                osrfAppSessionStatus(
                        ctx->session,
@@ -546,6 +548,8 @@ int rollbackSavepoint ( osrfMethodContext* ctx ) {
                return -1;
        }
 
+       char* spName = jsonObjectToSimpleString(jsonObjectGetIndex(ctx->params, 0));
+
        dbi_result result = dbi_conn_queryf(writehandle, "ROLLBACK TO SAVEPOINT \"%s\";", spName);
        if (!result) {
                osrfLogError(
@@ -556,12 +560,14 @@ int rollbackSavepoint ( osrfMethodContext* ctx ) {
                        osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )
                );
                osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, "osrfMethodException", ctx->request, "Error rolling back savepoint" );
+               free(spName);
                return -1;
        } else {
                jsonObject* ret = jsonNewObject(spName);
                osrfAppRespondComplete( ctx, ret );
                jsonObjectFree(ret);
        }
+       free(spName);
        return 0;
 }
 
@@ -922,10 +928,18 @@ static jsonObject* doCreate(osrfMethodContext* ctx, int* err ) {
                        id = buffer_release(_id);
                }
 
-               if (    !options
-                       || !jsonObjectGetKeyConst( options, "quiet")
-                       || strcmp( jsonObjectToSimpleString(jsonObjectGetKeyConst( options, "quiet")), "true" )
-               ) {
+               // Find quietness specification, if present
+               char* quiet_str = NULL;
+               if ( options ) {
+                       const jsonObject* quiet_obj = jsonObjectGetKeyConst( options, "quiet" );
+                       if( quiet_obj )
+                               quiet_str = jsonObjectToSimpleString( quiet_obj );
+               }
+
+               if( quiet_str && !strcmp( quiet_str, "true" )) {  // if quietness is specified
+                       obj = jsonNewObject(id);
+               }
+               else {
 
                        jsonObject* fake_params = jsonParseString("[]");
                        jsonObjectPush(fake_params, jsonParseString("{}"));
@@ -947,11 +961,9 @@ static jsonObject* doCreate(osrfMethodContext* ctx, int* err ) {
 
                        jsonObjectFree( list );
                        jsonObjectFree( fake_params );
-
-               } else {
-                       obj = jsonNewObject(id);
                }
 
+               if(quiet_str) free(quiet_str);
                free(id);
        }
 
@@ -988,6 +1000,8 @@ static jsonObject* doRetrieve(osrfMethodContext* ctx, int* err ) {
                jsonParseString(id)
        );
 
+       free(id);
+
        if (order_hash) jsonObjectPush(fake_params, jsonObjectClone(order_hash) );
 
        jsonObject* list = doFieldmapperSearch( ctx,meta, fake_params, err);
@@ -1010,11 +1024,19 @@ static char* jsonNumberToDBString ( osrfHash* field, const jsonObject* value ) {
 
        if ( !strncmp(osrfHashGet(field, "datatype"), "INT", (size_t)3) ) {
                if (value->type == JSON_NUMBER) buffer_fadd( val_buf, "%ld", (long)jsonObjectGetNumber(value) );
-               else buffer_fadd( val_buf, "%ld", atol(jsonObjectToSimpleString(value)) );
+               else {
+                       char* val_str = jsonObjectToSimpleString(value);
+                       buffer_fadd( val_buf, "%ld", atol(val_str) );
+                       free(val_str);
+               }
 
        } else if ( !strcmp(osrfHashGet(field, "datatype"), "NUMERIC") ) {
                if (value->type == JSON_NUMBER) buffer_fadd( val_buf, "%f",  jsonObjectGetNumber(value) );
-               else buffer_fadd( val_buf, "%f", atof(jsonObjectToSimpleString(value)) );
+               else {
+                       char* val_str = jsonObjectToSimpleString(value);
+                       buffer_fadd( val_buf, "%f", atof(val_str) );
+                       free(val_str);
+               }
        }
 
        return buffer_release(val_buf);
@@ -1160,10 +1182,12 @@ static char* searchFieldTransform (const char* class, osrfHash* field, const jso
                        } else {
                                osrfLogError(OSRF_LOG_MARK, "%s: Error quoting key string [%s]", MODULENAME, val);
                            free(field_transform);
+                                       free(val);
                                buffer_free(sql_buf);
                                return NULL;
                }
-           }
+                               free(val);
+                       }
 
         }
 
@@ -1414,15 +1438,9 @@ static char* searchJOIN ( const jsonObject* join_hash, osrfHash* leftmeta ) {
 
                char* class = osrfHashGet(idlClass, "classname");
 
-               char* table = getSourceDefinition(idlClass);
-               char* type = jsonObjectToSimpleString( jsonObjectGetKeyConst( snode->item, "type" ) );
-               char* filter_op = jsonObjectToSimpleString( jsonObjectGetKeyConst( snode->item, "filter_op" ) );
                char* fkey = jsonObjectToSimpleString( jsonObjectGetKeyConst( snode->item, "fkey" ) );
                char* field = jsonObjectToSimpleString( jsonObjectGetKeyConst( snode->item, "field" ) );
 
-               const jsonObject* filter = jsonObjectGetKeyConst( snode->item, "filter" );
-               const jsonObject* join_filter = jsonObjectGetKeyConst( snode->item, "join" );
-
                if (field && !fkey) {
                        fkey = (char*)oilsIDLFindPath("/%s/links/%s/key", class, field);
                        if (!fkey) {
@@ -1435,7 +1453,7 @@ static char* searchJOIN ( const jsonObject* join_hash, osrfHash* leftmeta ) {
                                        leftclass
                                );
                                buffer_free(join_buf);
-                               free(table);
+                               free(field);
                                return NULL;
                        }
                        fkey = strdup( fkey );
@@ -1452,7 +1470,7 @@ static char* searchJOIN ( const jsonObject* join_hash, osrfHash* leftmeta ) {
                                        class
                                );
                                buffer_free(join_buf);
-                               free(table);
+                               free(fkey);
                                return NULL;
                        }
                        field = strdup( field );
@@ -1499,12 +1517,12 @@ static char* searchJOIN ( const jsonObject* join_hash, osrfHash* leftmeta ) {
                                        class
                                );
                                buffer_free(join_buf);
-                               free(table);
                                return NULL;
                        }
 
                }
 
+               char* type = jsonObjectToSimpleString( jsonObjectGetKeyConst( snode->item, "type" ) );
                if (type) {
                        if ( !strcasecmp(type,"left") ) {
                                buffer_add(join_buf, " LEFT JOIN");
@@ -1518,11 +1536,15 @@ static char* searchJOIN ( const jsonObject* join_hash, osrfHash* leftmeta ) {
                } else {
                        buffer_add(join_buf, " INNER JOIN");
                }
+               free(type);
 
+               char* table = getSourceDefinition(idlClass);
                buffer_fadd(join_buf, " %s AS \"%s\" ON ( \"%s\".%s = \"%s\".%s", table, class, class, field, leftclass, fkey);
                free(table);
 
+               const jsonObject* filter = jsonObjectGetKeyConst( snode->item, "filter" );
                if (filter) {
+                       char* filter_op = jsonObjectToSimpleString( jsonObjectGetKeyConst( snode->item, "filter_op" ) );
                        if (filter_op) {
                                if (!strcasecmp("or",filter_op)) {
                                        buffer_add( join_buf, " OR " );
@@ -1536,18 +1558,18 @@ static char* searchJOIN ( const jsonObject* join_hash, osrfHash* leftmeta ) {
                        char* jpred = searchWHERE( filter, idlClass, AND_OP_JOIN );
                        buffer_fadd( join_buf, " %s", jpred );
                        free(jpred);
+                       free(filter_op);
                }
 
                buffer_add(join_buf, " ) ");
                
+               const jsonObject* join_filter = jsonObjectGetKeyConst( snode->item, "join" );
                if (join_filter) {
                        char* jpred = searchJOIN( join_filter, idlClass );
                        buffer_fadd( join_buf, " %s", jpred );
                        free(jpred);
                }
 
-               free(type);
-               free(filter_op);
                free(fkey);
                free(field);
        }
@@ -2227,7 +2249,9 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf
 
                jsonObjectIterator* select_itr = jsonNewObjectIterator( snode->item );
                while ( (node = jsonObjectIteratorNext( select_itr )) ) {
-                       osrfHash* field = osrfHashGet( osrfHashGet( idlClass, "fields" ), jsonObjectToSimpleString(node->item) );
+                       char* item_str = jsonObjectToSimpleString(node->item);
+                       osrfHash* field = osrfHashGet( osrfHashGet( idlClass, "fields" ), item_str );
+                       free(item_str);
                        char* fname = osrfHashGet(field, "name");
 
                        if (!field) continue;
@@ -2546,6 +2570,7 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta,
                                char* pkey_val = jsonObjectToSimpleString( jsonObjectGetIndex( obj, pkey_pos ) );
                                if ( osrfHashGet( dedup, pkey_val ) ) {
                                        jsonObjectFree(obj);
+                                       free(pkey_val);
                                } else {
                                        osrfHashSet( dedup, pkey_val, pkey_val );
                                        jsonObjectPush(res_list, obj);