[open-ils-commits] r8589 - trunk/Open-ILS/src/c-apps
svn at svn.open-ils.org
svn at svn.open-ils.org
Sun Feb 3 14:36:48 EST 2008
Author: miker
Date: 2008-02-03 14:08:44 -0500 (Sun, 03 Feb 2008)
New Revision: 8589
Modified:
trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
Patch from Scott McKellar:
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.
Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c 2008-02-03 03:51:59 UTC (rev 8588)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c 2008-02-03 19:08:44 UTC (rev 8589)
@@ -463,8 +463,6 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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,11 +928,19 @@
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 @@
jsonObjectFree( list );
jsonObjectFree( fake_params );
-
- } else {
- obj = jsonNewObject(id);
}
+ if(quiet_str) free(quiet_str);
free(id);
}
@@ -988,6 +1000,8 @@
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 @@
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 @@
} 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 @@
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 @@
leftclass
);
buffer_free(join_buf);
- free(table);
+ free(field);
return NULL;
}
fkey = strdup( fkey );
@@ -1452,7 +1470,7 @@
class
);
buffer_free(join_buf);
- free(table);
+ free(fkey);
return NULL;
}
field = strdup( field );
@@ -1499,12 +1517,12 @@
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 @@
} 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 @@
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 @@
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 @@
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);
More information about the open-ils-commits
mailing list