[Opensrf-commits] r1094 - trunk/src/srfsh

[svn at svn.open-ils.org]

Author: miker
Date: 2007-09-30 13:48:32 -0400 (Sun, 30 Sep 2007)
New Revision: 1094

Modified:
   trunk/src/srfsh/srfsh.c
Log:
Patch from Scott McKellar to improve const-correctness in srfsh:

This patch sprinkles a few const qualifiers over srfsh.c, and also
fixes a couple of things I stumbled across along the way.

The main purpose is to treat jsonObjects in a more const-correct
manner.  I introduced a new jsonObjectGetKeyConst function, and I
prepared for the day when jsonObjectGetString() returns a const
pointer instead of a non-const pointer.

This patch relies on a previous patch that defines the new
jsonObjectGetKeyConst function.

1. In handle_login() I changed hash to a const pointer, so that we
can assign the return value of jsonObjectGetString() to it.

2. Later in the same function, I rearranged the code a bit to ensure
that we always free a prior value of login_session before giving it
a new value.  The original code potentially leaks memory.

3. In the same area I changed authtoken to a const pointer.

4. In send_request() I eliminated an unnecessary test for the
non-nullness of omsg->_result_content, because we had just tested
it a few lines ago.

5. Also in send_request(): I introduced a layer of strdup() when
we get content from jsonObjectGetString().  This change concerns
more than just const-correctness.

The problem with the original code is that we get content sometimes
from jsonFormatString() and sometimes from jsonObjectGetString().
The former returns a malloc'ed buffer, which we need to free.  The
latter returns a pointer to a buffer owned by a jsonObject, and we
should not free it.  As it happens, we do free it.  If we got the
string from jsonObjectGetString(), then we leave the jsonObject with
an invalid pointer inside it.  If we free the jsonObject later, we'll
try to free that same buffer a second time.  Oops.

Another issue is that jsonObjectGetString() can return NULL, which
we should handle more carefully.

These problems occur in two different places in the same function.
I fixed them both the same way, with some differences in the details.



Modified: trunk/src/srfsh/srfsh.c
===================================================================
--- trunk/src/srfsh/srfsh.c	2007-09-30 14:03:19 UTC (rev 1093)
+++ trunk/src/srfsh/srfsh.c	2007-09-30 17:48:32 UTC (rev 1094)
@@ -356,7 +356,7 @@
 		sprintf( buf, login_text, username );
 		parse_request(buf);
 
-		char* hash;
+		const char* hash;
 		if(last_result && last_result->_result_content) {
 			jsonObject* r = last_result->_result_content;
 			hash = jsonObjectGetString(r);
@@ -388,17 +388,21 @@
 		parse_request( argbuf->buf );
 		buffer_free(argbuf);
 
-		jsonObject* x = last_result->_result_content;
+		if( login_session != NULL )
+			free( login_session );
+
+		const jsonObject* x = last_result->_result_content;
 		double authtime = 0;
 		if(x) {
-			char* authtoken = jsonObjectGetString(
-					jsonObjectGetKey(jsonObjectGetKey(x,"payload"), "authtoken"));
+			const char* authtoken = jsonObjectGetString(
+					jsonObjectGetKeyConst(jsonObjectGetKeyConst(x,"payload"), "authtoken"));
 			authtime  = jsonObjectGetNumber(
-					jsonObjectGetKey(jsonObjectGetKey(x,"payload"), "authtime"));
-			if(authtoken) {
-				free(login_session);
+					jsonObjectGetKeyConst(jsonObjectGetKeyConst(x,"payload"), "authtime"));
+
+			if(authtoken)
 				login_session = strdup(authtoken);
-			} else login_session = NULL;
+			else
+				login_session = NULL;
 		}
 		else login_session = NULL;
 
@@ -607,14 +611,18 @@
 	
 				char* content;
 	
-				if( pretty_print && omsg->_result_content ) {
+				if( pretty_print ) {
 					char* j = jsonObjectToJSON(omsg->_result_content);
 					//content = json_printer(j); 
 					content = jsonFormatString(j);
 					free(j);
-				} else
-					content = jsonObjectGetString(omsg->_result_content);
-	
+				} else {
+					const char * temp_content = jsonObjectGetString(omsg->_result_content);
+					if( ! temp_content )
+						temp_content = "[null]";
+					content = strdup( temp_content );
+				}
+				
 				printf( "\nReceived Data: %s\n", content ); 
 				free(content);
 	
@@ -645,9 +653,14 @@
 					//content = json_printer(j); 
 					content = jsonFormatString(j);
 					free(j);
-				} else
-					content = jsonObjectGetString(omsg->_result_content);
-	
+				} else {
+					const char * temp_content = jsonObjectGetString(omsg->_result_content);
+					if( temp_content )
+						content = strdup( temp_content );
+					else
+						content = NULL;
+				}
+
 				buffer_add( resp_buffer, "\nReceived Data: " ); 
 				buffer_add( resp_buffer, content );
 				buffer_add( resp_buffer, "\n" );