[Opensrf-commits] r1320 - trunk/src/gateway

[svn at svn.open-ils.org]

Author: erickson
Date: 2008-05-15 09:36:32 -0400 (Thu, 15 May 2008)
New Revision: 1320

Modified:
   trunk/src/gateway/apachetools.c
Log:
Patch from Scott McKellar:

This patch is mostly a performance tweak, but also tidies up a few
things.

In apacheParseParms() we load POST data and GET data into the same
buffer, with the GET data coming first.  However the old code loads 
the POST data first.  If there is also some GET data, we juggle some 
buffers in order to get the GET and POST data into the right order.

The new code loads the GET data first, and then appends the POST
data onto it.  Besides being simpler, the new code avoids a layer
of copying, as well as and two round trips through malloc() and 
free().

Other details:

1. I rearranged the declarations of the variables sarray, buffer,
key, and val, so as to narrow their scope.  In the case of sarray
this rearrangement avoids a potential memory leak in the case of an
error exit (where the POST data is excessive).

2. I append a terminal null to the input buffer instead of using
memset() to fill the entire buffer.

3. Since the bread variable is a long rather than an int, I corrected
the format specifier accordingly in one of the debug messages.

4. I eliminated some redundant casts in a couple of calls to
ap_unescape(), since the variables so cast are already the right
types.

5. The final debug message was being issued only when sarray was
not NULL.  However at this spot sarray cannot be NULL anyway, so I
eliminated the test of sarray.




Modified: trunk/src/gateway/apachetools.c
===================================================================
--- trunk/src/gateway/apachetools.c	2008-05-07 17:26:26 UTC (rev 1319)
+++ trunk/src/gateway/apachetools.c	2008-05-15 13:36:32 UTC (rev 1320)
@@ -4,16 +4,9 @@
 
 	if( r == NULL ) return NULL;
 
-	char* arg = r->args;			/* url query string */
+	char* arg = NULL;
 	apr_pool_t *p = r->pool;	/* memory pool */
-	osrfStringArray* sarray		= osrfNewStringArray(12); /* method parameters */
 
-	growing_buffer* buffer		= NULL;	/* POST data */
-	growing_buffer* tmp_buf		= NULL;	/* temp buffer */
-
-	char* key						= NULL;	/* query item name */
-	char* val						= NULL;	/* query item value */
-
 	/* gather the post args and append them to the url query string */
 	if( !strcmp(r->method,"POST") ) {
 
@@ -23,15 +16,21 @@
 
 		if(ap_should_client_block(r)) {
 
-			char body[1025];
-			memset(body,0,sizeof(body));
-			buffer = buffer_init(1025);
+			growing_buffer* buffer = buffer_init(1025);
 
+			/* Start with url query string, if any */
+			
+			if(r->args && r->args[0])
+				buffer_add(buffer, r->args);
 
+			char body[1025];
+
 			osrfLogDebug(OSRF_LOG_MARK, "gateway client has post data, reading...");
-	
+
+			/* Append POST data */
+			
 			long bread;
-			while( (bread = ap_get_client_block(r, body, 1024)) ) {
+			while( (bread = ap_get_client_block(r, body, sizeof(body) - 1)) ) {
 
 				if(bread < 0) {
 					osrfLogInfo(OSRF_LOG_MARK, 
@@ -39,14 +38,12 @@
 					break;
 				}
 
+				body[bread] = '\0';
 				buffer_add( buffer, body );
-				memset(body,0,sizeof(body));
 
 				osrfLogDebug(OSRF_LOG_MARK, 
-					"gateway read %d bytes: %d bytes of data so far", bread, buffer->n_used);
+					"gateway read %ld bytes: %d bytes of data so far", bread, buffer->n_used);
 
-				if(buffer->n_used == 0) break;
-
 				if(buffer->n_used > APACHE_TOOLS_MAX_POST_SIZE) {
 					osrfLogError(OSRF_LOG_MARK, "gateway received POST larger "
 						"than %d bytes. dropping request", APACHE_TOOLS_MAX_POST_SIZE);
@@ -57,20 +54,10 @@
 
 			osrfLogDebug(OSRF_LOG_MARK, "gateway done reading post data");
 	
-			if(arg && arg[0]) {
-
-				tmp_buf = buffer_init(1024);
-				buffer_add(tmp_buf,arg);
-				buffer_add(tmp_buf,buffer->buf);
-				arg = (char*) apr_pstrdup(p, tmp_buf->buf);
-				buffer_free(tmp_buf);
-
-			} else if(buffer->n_used > 0){
-					arg = (char*) apr_pstrdup(p, buffer->buf);
-
-			} else { 
+			if(buffer->n_used > 0)
+				arg = apr_pstrdup(p, buffer->buf);
+			else
 				arg = NULL; 
-			}
 
 			buffer_free(buffer);
 		}
@@ -83,17 +70,25 @@
 		return NULL;
 	}
 
-
 	osrfLogDebug(OSRF_LOG_MARK, "parsing URL params from post/get request data: %s", arg);
+	
+	osrfStringArray* sarray		= osrfNewStringArray(12); /* method parameters */
 	int sanity = 0;
+	char* key					= NULL;	/* query item name */
+	char* val					= NULL;	/* query item value */
+
+	/* Parse the post/get request data into a series of name/value pairs.   */
+	/* Load each name into an even-numbered slot of an osrfStringArray, and */
+	/* the corresponding value into the following odd-numbered slot.        */
+
 	while( arg && (val = ap_getword(p, (const char**) &arg, '&'))) {
 
 		key = ap_getword(r->pool, (const char**) &val, '=');
 		if(!key || !key[0])
 			break;
 
-		ap_unescape_url((char*)key);
-		ap_unescape_url((char*)val);
+		ap_unescape_url(key);
+		ap_unescape_url(val);
 
 		osrfLogDebug(OSRF_LOG_MARK, "parsed URL params %s=%s", key, val);
 
@@ -109,9 +104,8 @@
 
 	}
 
-	if(sarray)
-		osrfLogDebug(OSRF_LOG_MARK, 
-			"Apache tools parsed %d params key/values", sarray->size / 2 );
+	osrfLogDebug(OSRF_LOG_MARK,
+		"Apache tools parsed %d params key/values", sarray->size / 2 );
 
 	return sarray;
 }