[open-ils-commits] r8708 - trunk/Open-ILS/src/c-apps

[svn at svn.open-ils.org]

Author: miker
Date: 2008-02-11 07:52:04 -0500 (Mon, 11 Feb 2008)
New Revision: 8708

Modified:
   trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
Patch from Scott McKellar which plugs a couple of memory leaks, and
applies some minor optimizations, as much for clarity as for
performance.

1. In buildSELECT() we were leaking defaultselhash in the case of an
early return.

2. In doFieldmapperSearch() we were leaking flesh_blob in the case
of an early return.

3. In doFieldmapperSearch() I rearranged the logic a bit.  First,
I performed a single search of meta to get a method type and saved
the result for reuse, instead of performing the identical search
repeatedly.  Second, I turned a series of ifs into a series of
if/elses.  That way we stop searching when we find a match.  More
importantly, the if/else structure makes it more clear to the reader
that we're really just branching on method type in a case structure.

This latter change requires that none of the branches changes the
contents of ctx->method->userData.  So far as I can tell by tracing
out all the branches, this condition is satisfied, as one would
intuitively expect.

4. Also in doFieldmapperSearch(): I increased the size of the
growing_buffer sel_list from 16 characters to 64.  Since the
formatted string is at least 13 characters long, depending on the
length of the class name and primary key, I suspect that 16
characters will almost never be big enough.  Even 64 characters
might be too short.  I don't know how long the values typically
are in practice.



Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c	2008-02-11 05:04:23 UTC (rev 8707)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c	2008-02-11 12:52:04 UTC (rev 8708)
@@ -623,21 +623,27 @@
 	
 	int err = 0;
 
+	const char* methodtype = osrfHashGet(meta, "methodtype");
 	jsonObject * obj = NULL;
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "create"))
+
+	if (!strcmp(methodtype, "create")) {
 		obj = doCreate(ctx, &err);
-
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "retrieve"))
+		osrfAppRespondComplete( ctx, obj );
+	}
+	else if (!strcmp(methodtype, "retrieve")) {
 		obj = doRetrieve(ctx, &err);
-
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "update"))
+		osrfAppRespondComplete( ctx, obj );
+	}
+	else if (!strcmp(methodtype, "update")) {
 		obj = doUpdate(ctx, &err);
-
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "delete"))
+		osrfAppRespondComplete( ctx, obj );
+	}
+	else if (!strcmp(methodtype, "delete")) {
 		obj = doDelete(ctx, &err);
+		osrfAppRespondComplete( ctx, obj );
+	}
+	else if (!strcmp(methodtype, "search")) {
 
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "search")) {
-
 		obj = doFieldmapperSearch(ctx, class_obj, ctx->params, &err);
 		if(err) return err;
 
@@ -649,7 +655,7 @@
 		jsonObjectIteratorFree(itr);
 		osrfAppRespondComplete( ctx, NULL );
 
-	} else if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "id_list")) {
+	} else if (!strcmp(methodtype, "id_list")) {
 
 		jsonObject* _p = jsonObjectClone( ctx->params );
 		if (jsonObjectGetIndex( _p, 1 )) {
@@ -659,7 +665,7 @@
 			jsonObjectSetIndex( _p, 1, jsonParseString("{}") );
 		}
 
-		growing_buffer* sel_list = buffer_init(16);
+		growing_buffer* sel_list = buffer_init(64);
 		buffer_fadd(sel_list, "{ \"%s\":[\"%s\"] }", osrfHashGet( class_obj, "classname" ), osrfHashGet( class_obj, "primarykey" ));
 		char* _s = buffer_release(sel_list);
 
@@ -2314,6 +2320,7 @@
 				"Severe query error -- see error log for more details"
 			);
 		buffer_free(sql_buf);
+		if(defaultselhash) jsonObjectFree(defaultselhash);
 		return NULL;
 	} else {
 		buffer_add(sql_buf, pred);
@@ -2750,6 +2757,7 @@
 							osrfStringArrayFree(link_fields);
 							jsonObjectIteratorFree(itr);
 							jsonObjectFree(res_list);
+							jsonObjectFree(flesh_blob);
 							return jsonNULL;
 						}