From 00d63643433873a628affd1fdbad4fcb57b91319 Mon Sep 17 00:00:00 2001
From: miker <miker@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Date: Mon, 11 Feb 2008 12:52:04 +0000
Subject: [PATCH 1/1] 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.



git-svn-id: svn://svn.open-ils.org/ILS/trunk@8708 dcc99617-32d9-48b4-a31d-7c20da2025e4
---
 Open-ILS/src/c-apps/oils_cstore.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/Open-ILS/src/c-apps/oils_cstore.c b/Open-ILS/src/c-apps/oils_cstore.c
index bc619f5c62..0be1eb7688 100644
--- a/Open-ILS/src/c-apps/oils_cstore.c
+++ b/Open-ILS/src/c-apps/oils_cstore.c
@@ -623,20 +623,26 @@ int dispatchCRUDMethod ( osrfMethodContext* ctx ) {
 	
 	int err = 0;
 
+	const char* methodtype = osrfHashGet(meta, "methodtype");
 	jsonObject * obj = NULL;
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "create"))
-		obj = doCreate(ctx, &err);
 
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "retrieve"))
+	if (!strcmp(methodtype, "create")) {
+		obj = doCreate(ctx, &err);
+		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);
-
-	if (!strcmp( (char*)osrfHashGet(meta, "methodtype"), "search")) {
+		osrfAppRespondComplete( ctx, obj );
+	}
+	else if (!strcmp(methodtype, "search")) {
 
 		obj = doFieldmapperSearch(ctx, class_obj, ctx->params, &err);
 		if(err) return err;
@@ -649,7 +655,7 @@ int dispatchCRUDMethod ( osrfMethodContext* ctx ) {
 		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 @@ int dispatchCRUDMethod ( osrfMethodContext* ctx ) {
 			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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf
 				"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 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta,
 							osrfStringArrayFree(link_fields);
 							jsonObjectIteratorFree(itr);
 							jsonObjectFree(res_list);
+							jsonObjectFree(flesh_blob);
 							return jsonNULL;
 						}
 
-- 
2.43.2