Adding comments and tinkering with white space.
authorscottmk <scottmk@9efc2488-bf62-4759-914b-345cdb29e865>
Wed, 11 Aug 2010 02:58:25 +0000 (02:58 +0000)
committerscottmk <scottmk@9efc2488-bf62-4759-914b-345cdb29e865>
Wed, 11 Aug 2010 02:58:25 +0000 (02:58 +0000)
No substantive changes.

M    src/libopensrf/osrf_prefork.c

git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1994 9efc2488-bf62-4759-914b-345cdb29e865

src/libopensrf/osrf_prefork.c

index 69ccc07..6531c2d 100644 (file)
@@ -88,16 +88,16 @@ static int prefork_simple_init( prefork_simple* prefork, transport_client* clien
        int max_requests, int min_children, int max_children );
 static prefork_child* launch_child( prefork_simple* forker );
 static void prefork_launch_children( prefork_simple* forker );
-static void prefork_run(prefork_simple* forker);
+static void prefork_run( prefork_simple* forker );
 static void add_prefork_child( prefork_simple* forker, prefork_child* child );
 
 static void del_prefork_child( prefork_simple* forker, pid_t pid );
 static void check_children( prefork_simple* forker, int forever );
-static int  prefork_child_process_request(prefork_child*, char* data);
-static int prefork_child_init_hook(prefork_child*);
+static int  prefork_child_process_request( prefork_child*, char* data );
+static int prefork_child_init_hook( prefork_child* );
 static prefork_child* prefork_child_init( prefork_simple* forker,
-               int read_data_fd, int write_data_fd,
-               int read_status_fd, int write_status_fd );
+       int read_data_fd, int write_data_fd,
+       int read_status_fd, int write_status_fd );
 
 /* listens on the 'data_to_child' fd and wait for incoming data */
 static void prefork_child_wait( prefork_child* child );
@@ -115,7 +115,7 @@ static void sigchld_handler( int sig );
 */
 int osrf_prefork_run( const char* appname ) {
 
-       if(!appname) {
+       if( !appname ) {
                osrfLogError( OSRF_LOG_MARK, "osrf_prefork_run requires an appname to run!");
                return -1;
        }
@@ -127,67 +127,74 @@ int osrf_prefork_run( const char* appname ) {
        int minc = 3;
        int kalive = 5;
 
-       osrfLogInfo( OSRF_LOG_MARK, "Loading config in osrf_forker for app %s", appname);
-
-       char* max_req      = osrf_settings_host_value("/apps/%s/unix_config/max_requests", appname);
-       char* min_children = osrf_settings_host_value("/apps/%s/unix_config/min_children", appname);
-       char* max_children = osrf_settings_host_value("/apps/%s/unix_config/max_children", appname);
-       char* keepalive    = osrf_settings_host_value("/apps/%s/keepalive", appname);
-
-       if(!keepalive) osrfLogWarning( OSRF_LOG_MARK, "Keepalive is not defined, assuming %d", kalive);
-       else kalive = atoi(keepalive);
-
-       if(!max_req) osrfLogWarning( OSRF_LOG_MARK, "Max requests not defined, assuming %d", maxr);
-       else maxr = atoi(max_req);
-
-       if(!min_children) osrfLogWarning( OSRF_LOG_MARK,
-                       "Min children not defined, assuming %d", minc);
-       else minc = atoi(min_children);
-
-       if(!max_children) osrfLogWarning( OSRF_LOG_MARK,
-                       "Max children not defined, assuming %d", maxc);
-       else maxc = atoi(max_children);
-
-       free(keepalive);
-       free(max_req);
-       free(min_children);
-       free(max_children);
+       // Get configuration settings
+       osrfLogInfo( OSRF_LOG_MARK, "Loading config in osrf_forker for app %s", appname );
+
+       char* max_req      = osrf_settings_host_value( "/apps/%s/unix_config/max_requests", appname );
+       char* min_children = osrf_settings_host_value( "/apps/%s/unix_config/min_children", appname );
+       char* max_children = osrf_settings_host_value( "/apps/%s/unix_config/max_children", appname );
+       char* keepalive    = osrf_settings_host_value( "/apps/%s/keepalive", appname );
+
+       if( !keepalive )
+               osrfLogWarning( OSRF_LOG_MARK, "Keepalive is not defined, assuming %d", kalive );
+       else
+               kalive = atoi( keepalive );
+
+       if( !max_req )
+               osrfLogWarning( OSRF_LOG_MARK, "Max requests not defined, assuming %d", maxr );
+       else
+               maxr = atoi( max_req );
+
+       if( !min_children )
+               osrfLogWarning( OSRF_LOG_MARK, "Min children not defined, assuming %d", minc );
+       else
+               minc = atoi( min_children );
+
+       if( !max_children )
+               osrfLogWarning( OSRF_LOG_MARK, "Max children not defined, assuming %d", maxc );
+       else
+               maxc = atoi( max_children );
+
+       free( keepalive );
+       free( max_req );
+       free( min_children );
+       free( max_children );
        /* --------------------------------------------------- */
 
-       char* resc = va_list_to_string("%s_listener", appname);
+       char* resc = va_list_to_string( "%s_listener", appname );
 
        // Make sure that we haven't already booted
-       if(!osrfSystemBootstrapClientResc( NULL, NULL, resc )) {
-               osrfLogError( OSRF_LOG_MARK, "Unable to bootstrap client for osrf_prefork_run()");
-               free(resc);
+       if( !osrfSystemBootstrapClientResc( NULL, NULL, resc )) {
+               osrfLogError( OSRF_LOG_MARK, "Unable to bootstrap client for osrf_prefork_run()" );
+               free( resc );
                return -1;
        }
 
-       free(resc);
+       free( resc );
 
        prefork_simple forker;
 
-       if( prefork_simple_init( &forker, osrfSystemGetTransportClient(), maxr, minc, maxc ) ) {
+       if( prefork_simple_init( &forker, osrfSystemGetTransportClient(), maxr, minc, maxc )) {
                osrfLogError( OSRF_LOG_MARK,
-                               "osrf_prefork_run() failed to create prefork_simple object" );
+                       "osrf_prefork_run() failed to create prefork_simple object" );
                return -1;
        }
 
        // Finish initializing the prefork_simple.
-       forker.appname   = strdup(appname);
+       forker.appname   = strdup( appname );
        forker.keepalive = kalive;
 
        // Spawn the children; put them in the idle list.
        prefork_launch_children( &forker );
 
        // Tell the router that you're open for business.
-       osrf_prefork_register_routers(appname);
+       osrf_prefork_register_routers( appname );
 
        // Sit back and let the requests roll in
-       osrfLogInfo( OSRF_LOG_MARK, "Launching osrf_forker for app %s", appname);
+       osrfLogInfo( OSRF_LOG_MARK, "Launching osrf_forker for app %s", appname );
        prefork_run( &forker );
 
-       osrfLogWarning( OSRF_LOG_MARK, "prefork_run() returned - how??");
+       osrfLogWarning( OSRF_LOG_MARK, "prefork_run() returned - how??" );
        prefork_clear( &forker );
        return 0;
 }
@@ -218,38 +225,55 @@ static void osrf_prefork_send_router_registration(
 
        // Clean up
        message_free( msg );
-       free(jid);
+       free( jid );
 }
 
-/* parses a single "complex" router configuration chunk */
-// Called only by the parent process
-static void osrf_prefork_parse_router_chunk(const char* appname, const jsonObject* routerChunk) {
+/**
+       @brief Register with a router, or not, according to some config settings.
+       @param appname Name of the application
+       @param RouterChunk A representation of part of the config file.
+
+       Parse a "complex" router configuration chunk.
 
-       const char* routerName = jsonObjectGetString(jsonObjectGetKeyConst(routerChunk, "name"));
-       const char* domain = jsonObjectGetString(jsonObjectGetKeyConst(routerChunk, "domain"));
-       const jsonObject* services = jsonObjectGetKeyConst(routerChunk, "services");
-       osrfLogDebug(OSRF_LOG_MARK, "found router config with domain %s and name %s",
-                       routerName, domain);
+       Examine the services listed for a given router (normally in opensrf_core.xml).  If
+       there is an entry for this service, or if there are @em no services listed, then
+       register with this router.  Otherwise don't.
+
+       Called only by the parent process.
+*/
+static void osrf_prefork_parse_router_chunk( const char* appname, const jsonObject* routerChunk ) {
+
+       const char* routerName = jsonObjectGetString( jsonObjectGetKeyConst( routerChunk, "name" ));
+       const char* domain = jsonObjectGetString( jsonObjectGetKeyConst( routerChunk, "domain" ));
+       const jsonObject* services = jsonObjectGetKeyConst( routerChunk, "services" );
+       osrfLogDebug( OSRF_LOG_MARK, "found router config with domain %s and name %s",
+               routerName, domain );
 
        if( services && services->type == JSON_HASH ) {
-               osrfLogDebug(OSRF_LOG_MARK, "investigating router information...");
-               const jsonObject* service_obj = jsonObjectGetKeyConst(services, "service");
+               osrfLogDebug( OSRF_LOG_MARK, "investigating router information..." );
+               const jsonObject* service_obj = jsonObjectGetKeyConst( services, "service" );
                if( !service_obj )
                        ;    // do nothing (shouldn't happen)
                else if( JSON_ARRAY == service_obj->type ) {
+                       // There are multiple services listed.  Register with this router
+                       // if and only if this service is on the list.
                        int j;
-                       for(j = 0; j < service_obj->size; j++ ) {
-                               const char* service = jsonObjectGetString(jsonObjectGetIndex(service_obj, j));
+                       for( j = 0; j < service_obj->size; j++ ) {
+                               const char* service = jsonObjectGetString( jsonObjectGetIndex( service_obj, j ));
                                if( service && !strcmp( appname, service ))
-                                       osrf_prefork_send_router_registration(appname, routerName, domain);
+                                       osrf_prefork_send_router_registration( appname, routerName, domain );
                        }
                }
                else if( JSON_STRING == service_obj->type ) {
-                       if( !strcmp(appname, jsonObjectGetString( service_obj )) )
-                               osrf_prefork_send_router_registration(appname, routerName, domain);
+                       // There's only one service listed.  Register with this router
+                       // if and only if this service is the one listed.
+                       if( !strcmp( appname, jsonObjectGetString( service_obj )) )
+                               osrf_prefork_send_router_registration( appname, routerName, domain );
                }
        } else {
-               osrf_prefork_send_router_registration(appname, routerName, domain);
+               // This router is not restricted to any set of services,
+               // so go ahead and register with it.
+               osrf_prefork_send_router_registration( appname, routerName, domain );
        }
 }
 
@@ -261,24 +285,24 @@ static void osrf_prefork_parse_router_chunk(const char* appname, const jsonObjec
 */
 static void osrf_prefork_register_routers( const char* appname ) {
 
-       jsonObject* routerInfo = osrfConfigGetValueObject(NULL, "/routers/router");
+       jsonObject* routerInfo = osrfConfigGetValueObject( NULL, "/routers/router" );
 
        int i;
-       for(i = 0; i < routerInfo->size; i++) {
-               const jsonObject* routerChunk = jsonObjectGetIndex(routerInfo, i);
+       for( i = 0; i < routerInfo->size; i++ ) {
+               const jsonObject* routerChunk = jsonObjectGetIndex( routerInfo, i );
 
-               if(routerChunk->type == JSON_STRING) {
+               if( routerChunk->type == JSON_STRING ) {
                        /* this accomodates simple router configs */
                        char* routerName = osrfConfigGetValue( NULL, "/router_name" );
-                       char* domain = osrfConfigGetValue(NULL, "/routers/router");
-                       osrfLogDebug(OSRF_LOG_MARK, "found simple router settings with router name %s",
-                                       routerName);
-                       osrf_prefork_send_router_registration(appname, routerName, domain);
+                       char* domain = osrfConfigGetValue( NULL, "/routers/router" );
+                       osrfLogDebug( OSRF_LOG_MARK, "found simple router settings with router name %s",
+                               routerName );
+                       osrf_prefork_send_router_registration( appname, routerName, domain );
 
                        free( routerName );
                        free( domain );
                } else {
-                       osrf_prefork_parse_router_chunk(appname, routerChunk);
+                       osrf_prefork_parse_router_chunk( appname, routerChunk );
                }
        }
 
@@ -297,39 +321,39 @@ static void osrf_prefork_register_routers( const char* appname ) {
        - Dynamically call an application-specific initialization routine
        - Change the command line as reported by ps
 */
-static int prefork_child_init_hook(prefork_child* child) {
+static int prefork_child_init_hook( prefork_child* child ) {
 
-       if(!child) return -1;
-       osrfLogDebug( OSRF_LOG_MARK, "Child init hook for child %d", child->pid);
+       if( !child ) return -1;
+       osrfLogDebug( OSRF_LOG_MARK, "Child init hook for child %d", child->pid );
 
        // Connect to cache server(s).
        osrfSystemInitCache();
-       char* resc = va_list_to_string("%s_drone", child->appname);
+       char* resc = va_list_to_string( "%s_drone", child->appname );
 
        // If we're a source-client, tell the logger now that we're a new process.
-       char* isclient = osrfConfigGetValue(NULL, "/client");
-       if( isclient && !strcasecmp(isclient,"true") )
-               osrfLogSetIsClient(1);
-       free(isclient);
+       char* isclient = osrfConfigGetValue( NULL, "/client" );
+       if( isclient && !strcasecmp( isclient,"true" ))
+               osrfLogSetIsClient( 1 );
+       free( isclient );
 
        // Remove traces of our parent's socket connection so we can have our own.
        osrfSystemIgnoreTransportClient();
 
        // Connect to Jabber
-       if(!osrfSystemBootstrapClientResc( NULL, NULL, resc )) {
-               osrfLogError( OSRF_LOG_MARK, "Unable to bootstrap client for osrf_prefork_run()");
-               free(resc);
+       if( !osrfSystemBootstrapClientResc( NULL, NULL, resc )) {
+               osrfLogError( OSRF_LOG_MARK, "Unable to bootstrap client for osrf_prefork_run()" );
+               free( resc );
                return -1;
        }
 
-       free(resc);
+       free( resc );
 
        // Dynamically call the application-specific initialization function
        // from a previously loaded shared library.
-       if( ! osrfAppRunChildInit(child->appname) ) {
-               osrfLogDebug(OSRF_LOG_MARK, "Prefork child_init succeeded\n");
+       if( ! osrfAppRunChildInit( child->appname )) {
+               osrfLogDebug( OSRF_LOG_MARK, "Prefork child_init succeeded\n" );
        } else {
-               osrfLogError(OSRF_LOG_MARK, "Prefork child_init failed\n");
+               osrfLogError( OSRF_LOG_MARK, "Prefork child_init failed\n" );
                return -1;
        }
 
@@ -338,30 +362,39 @@ static int prefork_child_init_hook(prefork_child* child) {
        return 0;
 }
 
-// Called only by a child process
-// Non-zero return code means that the child process has decided to terminate immediately,
-// without waiting for a DISCONNECT or max_requests.
-static int prefork_child_process_request(prefork_child* child, char* data) {
+/**
+       @brief Respond to a client request forwarded by the parent.
+       @param child Pointer to the state of the child process.
+       @param data Pointer to the raw XMPP message received from the parent.
+       @return 0 on success; non-zero means that the child process should clean itself up
+               and terminate immediately, presumably due to a fatal error condition.
+
+       Called only by a child process.
+*/
+static int prefork_child_process_request( prefork_child* child, char* data ) {
        if( !child ) return 0;
 
        transport_client* client = osrfSystemGetTransportClient();
 
-       if(!client_connected(client)) {
+       // Make sure that we're still connected to Jabber; reconnect if necessary.
+       if( !client_connected( client )) {
                osrfSystemIgnoreTransportClient();
-               osrfLogWarning(OSRF_LOG_MARK, "Reconnecting child to opensrf after disconnect...");
-               if(!osrf_system_bootstrap_client(NULL, NULL)) {
+               osrfLogWarning( OSRF_LOG_MARK, "Reconnecting child to opensrf after disconnect..." );
+               if( !osrf_system_bootstrap_client( NULL, NULL )) {
                        osrfLogError( OSRF_LOG_MARK,
-                               "Unable to bootstrap client in prefork_child_process_request()");
-                       sleep(1);
-                       osrf_prefork_child_exit(child);
+                               "Unable to bootstrap client in prefork_child_process_request()" );
+                       sleep( 1 );
+                       osrf_prefork_child_exit( child );
                }
        }
 
-       /* construct the message from the xml */
+       // Construct the message from the xml.
        transport_message* msg = new_message_from_xml( data );
 
-       osrfAppSession* session = osrf_stack_transport_handler(msg, child->appname);
-       if(!session) return 0;
+       // Respond to the transport message.  This is where method calls are buried.
+       osrfAppSession* session = osrf_stack_transport_handler( msg, child->appname );
+       if( !session )
+               return 0;
 
        int rc = session->panic;
 
@@ -373,10 +406,20 @@ static int prefork_child_process_request(prefork_child* child, char* data) {
        }
 
        if( session->stateless && session->state != OSRF_SESSION_CONNECTED ) {
+               // We're no longer connected to the client, which presumably means that
+               // we're done with this request.  Bail out.
                osrfAppSessionFree( session );
                return rc;
        }
 
+       // If we get this far, then the client has opened an application connection so that it
+       // can send multiple requests directly to the same server drone, bypassing the router
+       // and the listener.  For example, it may need to do a database transaction, requiring
+       // multiple method calls within the same database session.
+
+       // Hence we go into a loop, responding to successive requests from the same client, until
+       // either the client disconnects or an error occurs.
+
        osrfLogDebug( OSRF_LOG_MARK, "Entering keepalive loop for session %s", session->session_id );
        int keepalive = child->keepalive;
        int retval;
@@ -384,42 +427,48 @@ static int prefork_child_process_request(prefork_child* child, char* data) {
        time_t start;
        time_t end;
 
-       while(1) {
+       while( 1 ) {
 
-               osrfLogDebug(OSRF_LOG_MARK,
-                               "osrf_prefork calling queue_wait [%d] in keepalive loop", keepalive);
-               start   = time(NULL);
-               retval  = osrf_app_session_queue_wait(session, keepalive, &recvd);
-               end     = time(NULL);
+               // Respond to any input messages.  This is where the method calls are buried.
+               osrfLogDebug( OSRF_LOG_MARK,
+                       "osrf_prefork calling queue_wait [%d] in keepalive loop", keepalive );
+               start   = time( NULL );
+               retval  = osrf_app_session_queue_wait( session, keepalive, &recvd );
+               end     = time( NULL );
 
-               osrfLogDebug(OSRF_LOG_MARK, "Data received == %d", recvd);
+               osrfLogDebug( OSRF_LOG_MARK, "Data received == %d", recvd );
 
+               // Now we check a number of possible reasons to exit the loop.
+
+               // If the method call decided to terminate immediately,
+               // note that for future reference.
                if( session->panic )
                        rc = 1;
 
-               if(retval) {
-                       osrfLogError(OSRF_LOG_MARK, "queue-wait returned non-success %d", retval);
+               // If an error occurred when we tried to service the request, exit the loop.
+               if( retval ) {
+                       osrfLogError( OSRF_LOG_MARK, "queue-wait returned non-success %d", retval );
                        break;
                }
 
-               /* see if the client disconnected from us */
-               if(session->state != OSRF_SESSION_CONNECTED)
+               // If the client disconnected, exit the loop.
+               if( session->state != OSRF_SESSION_CONNECTED )
                        break;
 
-               /* if no data was reveived within the timeout interval */
+               // If we timed out while waiting for a request, exit the loop.
                if( !recvd && (end - start) >= keepalive ) {
-                       osrfLogInfo(OSRF_LOG_MARK,
-                                       "No request was received in %d seconds, exiting stateful session", keepalive);
+                       osrfLogInfo( OSRF_LOG_MARK,
+                               "No request was received in %d seconds, exiting stateful session", keepalive );
                        osrfAppSessionStatus(
-                                       session,
-                                       OSRF_STATUS_TIMEOUT,
-                                       "osrfConnectStatus",
-                                       0, "Disconnected on timeout" );
+                               session,
+                               OSRF_STATUS_TIMEOUT,
+                               "osrfConnectStatus",
+                               0, "Disconnected on timeout" );
 
                        break;
                }
 
-               // If the child process has decided to terminate immediately
+               // If the child process has decided to terminate immediately, exit the loop.
                if( rc )
                        break;
        }
@@ -444,17 +493,17 @@ static int prefork_simple_init( prefork_simple* prefork, transport_client* clien
 
        if( min_children > max_children ) {
                osrfLogError( OSRF_LOG_MARK,  "min_children (%d) is greater "
-                               "than max_children (%d)", min_children, max_children );
+                       "than max_children (%d)", min_children, max_children );
                return 1;
        }
 
        if( max_children > ABS_MAX_CHILDREN ) {
                osrfLogError( OSRF_LOG_MARK,  "max_children (%d) is greater than ABS_MAX_CHILDREN (%d)",
-                               max_children, ABS_MAX_CHILDREN );
+                       max_children, ABS_MAX_CHILDREN );
                return 1;
        }
 
-       osrfLogInfo(OSRF_LOG_MARK, "Prefork launching child with max_request=%d,"
+       osrfLogInfo( OSRF_LOG_MARK, "Prefork launching child with max_request=%d,"
                "min_children=%d, max_children=%d", max_requests, min_children, max_children );
 
        /* flesh out the struct */
@@ -492,12 +541,12 @@ static prefork_child* launch_child( prefork_simple* forker ) {
        int status_fd[2];
 
        // Set up the data and status pipes
-       if( pipe(data_fd) < 0 ) { /* build the data pipe*/
+       if( pipe( data_fd ) < 0 ) { /* build the data pipe*/
                osrfLogError( OSRF_LOG_MARK,  "Pipe making error" );
                return NULL;
        }
 
-       if( pipe(status_fd) < 0 ) {/* build the status pipe */
+       if( pipe( status_fd ) < 0 ) {/* build the status pipe */
                osrfLogError( OSRF_LOG_MARK,  "Pipe making error" );
                close( data_fd[1] );
                close( data_fd[0] );
@@ -505,11 +554,11 @@ static prefork_child* launch_child( prefork_simple* forker ) {
        }
 
        osrfLogInternal( OSRF_LOG_MARK, "Pipes: %d %d %d %d",
-                       data_fd[0], data_fd[1], status_fd[0], status_fd[1] );
+               data_fd[0], data_fd[1], status_fd[0], status_fd[1] );
 
        // Create and initialize a prefork_child for the new process
        prefork_child* child = prefork_child_init( forker, data_fd[0],
-                       data_fd[1], status_fd[0], status_fd[1] );
+               data_fd[1], status_fd[0], status_fd[1] );
 
        if( (pid=fork()) < 0 ) {
                osrfLogError( OSRF_LOG_MARK, "Forking Error" );
@@ -523,8 +572,8 @@ static prefork_child* launch_child( prefork_simple* forker ) {
 
        if( pid > 0 ) {  /* parent */
 
-               signal(SIGCHLD, sigchld_handler);
-               (forker->current_num_children)++;
+               signal( SIGCHLD, sigchld_handler );
+               ( forker->current_num_children )++;
                child->pid = pid;
 
                osrfLogDebug( OSRF_LOG_MARK, "Parent launched %d", pid );
@@ -536,18 +585,18 @@ static prefork_child* launch_child( prefork_simple* forker ) {
        else { /* child */
 
                osrfLogInternal( OSRF_LOG_MARK,
-                               "I am new child with read_data_fd = %d and write_status_fd = %d",
-                               child->read_data_fd, child->write_status_fd );
+                       "I am new child with read_data_fd = %d and write_status_fd = %d",
+                       child->read_data_fd, child->write_status_fd );
 
                child->pid = getpid();
                close( child->write_data_fd );
                close( child->read_status_fd );
 
                /* do the initing */
-               if( prefork_child_init_hook(child) == -1 ) {
-                       osrfLogError(OSRF_LOG_MARK,
-                               "Forker child going away because we could not connect to OpenSRF...");
-                       osrf_prefork_child_exit(child);
+               if( prefork_child_init_hook( child ) == -1 ) {
+                       osrfLogError( OSRF_LOG_MARK,
+                               "Forker child going away because we could not connect to OpenSRF..." );
+                       osrf_prefork_child_exit( child );
                }
 
                prefork_child_wait( child );      // Should exit without returning
@@ -558,14 +607,14 @@ static prefork_child* launch_child( prefork_simple* forker ) {
 
 /**
        @brief Terminate a child process.
-       @param child Pointer to the prefork_child representing the child process.
+       @param child Pointer to the prefork_child representing the child process (not used).
 
        Called only by child processes.  Dynamically call an application-specific shutdown
        function from a previously loaded shared library; then exit.
 */
-static void osrf_prefork_child_exit(prefork_child* child) {
+static void osrf_prefork_child_exit( prefork_child* child ) {
        osrfAppRunExitCode();
-       exit(0);
+       exit( 0 );
 }
 
 /**
@@ -575,7 +624,7 @@ static void osrf_prefork_child_exit(prefork_child* child) {
        Called only by the parent process (in order to become a parent).
 */
 static void prefork_launch_children( prefork_simple* forker ) {
-       if(!forker) return;
+       if( !forker ) return;
        int c = 0;
        while( c++ < forker->min_children )
                launch_child( forker );
@@ -588,7 +637,7 @@ static void prefork_launch_children( prefork_simple* forker ) {
        Set a boolean to be checked later.
 */
 static void sigchld_handler( int sig ) {
-       signal(SIGCHLD, sigchld_handler);
+       signal( SIGCHLD, sigchld_handler );
        child_dead = 1;
 }
 
@@ -612,7 +661,7 @@ void reap_children( prefork_simple* forker ) {
        // Bury the children so that they won't be zombies.  WNOHANG means that waitpid() returns
        // immediately if there are no waitable children, instead of waiting for more to die.
        // Ignore the return code of the child.  We don't do an autopsy.
-       while( (child_pid = waitpid(-1, NULL, WNOHANG)) > 0) {
+       while( (child_pid = waitpid( -1, NULL, WNOHANG )) > 0 ) {
                --forker->current_num_children;
                del_prefork_child( forker, child_pid );
        }
@@ -641,7 +690,7 @@ static void prefork_run( prefork_simple* forker ) {
 
        transport_message* cur_msg = NULL;
 
-       while(1) {
+       while( 1 ) {
 
                if( forker->first_child == NULL && forker->idle_list == NULL ) {/* no more children */
                        osrfLogWarning( OSRF_LOG_MARK, "No more children..." );
@@ -649,7 +698,7 @@ static void prefork_run( prefork_simple* forker ) {
                }
 
                // Wait indefinitely for an input message
-               osrfLogDebug( OSRF_LOG_MARK, "Forker going into wait for data...");
+               osrfLogDebug( OSRF_LOG_MARK, "Forker going into wait for data..." );
                cur_msg = client_recv( forker->connection, -1 );
 
                if( cur_msg == NULL )
@@ -659,7 +708,7 @@ static void prefork_run( prefork_simple* forker ) {
                const char* msg_data = cur_msg->msg_xml;
                if( ! msg_data || ! *msg_data ) {
                        osrfLogWarning( OSRF_LOG_MARK, "Received % message from %s, thread %",
-                                       (msg_data ? "empty" : "NULL"), cur_msg->sender, cur_msg->thread );
+                               (msg_data ? "empty" : "NULL"), cur_msg->sender, cur_msg->thread );
                        message_free( cur_msg );
                        continue;       // Message not usable; go on to the next one.
                }
@@ -669,7 +718,7 @@ static void prefork_run( prefork_simple* forker ) {
 
                while( ! honored ) {
 
-                       if(!no_recheck)
+                       if( !no_recheck )
                                check_children( forker, 0 );
                        no_recheck = 0;
 
@@ -690,19 +739,19 @@ static void prefork_run( prefork_simple* forker ) {
                                cur_child->next = NULL;
 
                                osrfLogInternal( OSRF_LOG_MARK,
-                                               "Searching for available child. cur_child->pid = %d", cur_child->pid );
+                                       "Searching for available child. cur_child->pid = %d", cur_child->pid );
                                osrfLogInternal( OSRF_LOG_MARK, "Current num children %d",
-                                               forker->current_num_children );
+                                       forker->current_num_children );
 
                                osrfLogDebug( OSRF_LOG_MARK, "forker sending data to %d", cur_child->pid );
                                osrfLogInternal( OSRF_LOG_MARK, "Writing to child fd %d",
-                                               cur_child->write_data_fd );
+                                       cur_child->write_data_fd );
 
-                               int written = write(cur_child->write_data_fd, msg_data, strlen(msg_data) + 1);
+                               int written = write( cur_child->write_data_fd, msg_data, strlen( msg_data ) + 1 );
                                if( written < 0 ) {
                                        // This child appears to be dead or unusable.  Discard it.
                                        osrfLogWarning( OSRF_LOG_MARK, "Write returned error %d: %s",
-                                                       errno, strerror( errno ) );
+                                               errno, strerror( errno ));
                                        kill( cur_child->pid, SIGKILL );
                                        del_prefork_child( forker, cur_child->pid );
                                        continue;
@@ -715,11 +764,11 @@ static void prefork_run( prefork_simple* forker ) {
 
                        /* if none available, add a new child if we can */
                        if( ! honored ) {
-                               osrfLogDebug( OSRF_LOG_MARK, "Not enough children, attempting to add...");
+                               osrfLogDebug( OSRF_LOG_MARK, "Not enough children, attempting to add..." );
 
                                if( forker->current_num_children < forker->max_children ) {
                                        osrfLogDebug( OSRF_LOG_MARK,  "Launching new child with current_num = %d",
-                                                       forker->current_num_children );
+                                               forker->current_num_children );
 
                                        launch_child( forker );  // Put a new child into the idle list
                                        if( forker->idle_list ) {
@@ -730,14 +779,14 @@ static void prefork_run( prefork_simple* forker ) {
                                                new_child->next = NULL;
 
                                                osrfLogDebug( OSRF_LOG_MARK, "Writing to new child fd %d : pid %d",
-                                                               new_child->write_data_fd, new_child->pid );
+                                                       new_child->write_data_fd, new_child->pid );
 
                                                int written = write(
-                                                               new_child->write_data_fd, msg_data, strlen(msg_data) + 1);
+                                                       new_child->write_data_fd, msg_data, strlen( msg_data ) + 1 );
                                                if( written < 0 ) {
                                                        // This child appears to be dead or unusable.  Discard it.
                                                        osrfLogWarning( OSRF_LOG_MARK, "Write returned error %d: %s",
-                                                                                       errno, strerror( errno ) );
+                                                               errno, strerror( errno ));
                                                        kill( cur_child->pid, SIGKILL );
                                                        del_prefork_child( forker, cur_child->pid );
                                                } else {
@@ -750,14 +799,14 @@ static void prefork_run( prefork_simple* forker ) {
                        }
 
                        if( !honored ) {
-                               osrfLogWarning( OSRF_LOG_MARK, "No children available, waiting...");
+                               osrfLogWarning( OSRF_LOG_MARK, "No children available, waiting..." );
                                check_children( forker, 1 );
                                // Tell the loop not to call check_children again, since we're calling it now
                                no_recheck = 1;
                        }
 
                        if( child_dead )
-                               reap_children(forker);
+                               reap_children( forker );
 
                } // end while( ! honored )
 
@@ -775,7 +824,6 @@ static void prefork_run( prefork_simple* forker ) {
        @brief See if any children have become available.
        @param forker Pointer to the prefork_simple that owns the children.
        @param forever Boolean: true if we should wait indefinitely.
-       @return 
 
        Call select() for all the children in the active list.  Read each active file
        descriptor and move the corresponding child to the idle list.
@@ -793,13 +841,13 @@ static void check_children( prefork_simple* forker, int forever ) {
                // processes, so there should be some active ones around.
                // If forever is false, then the children may all be idle, and that's okay.
                if( forever )
-                       osrfLogError( OSRF_LOG_MARK, "No active child processes to check" );
+                       osrfLogError( OSRF_LOG_MARK, "No active child processes to check" );
                return;
        }
 
        int select_ret;
        fd_set read_set;
-       FD_ZERO(&read_set);
+       FD_ZERO( &read_set );
        int max_fd = 0;
        int n;
 
@@ -812,18 +860,18 @@ static void check_children( prefork_simple* forker, int forever ) {
                cur_child = cur_child->next;
        } while( cur_child != forker->first_child );
 
-       FD_CLR(0,&read_set); /* just to be sure */
+       FD_CLR( 0, &read_set ); /* just to be sure */
 
        if( forever ) {
-               osrfLogWarning(OSRF_LOG_MARK,
-                               "We have no children available - waiting for one to show up...");
+               osrfLogWarning( OSRF_LOG_MARK,
+                       "We have no children available - waiting for one to show up..." );
 
-               if( (select_ret=select( max_fd + 1, &read_set, NULL, NULL, NULL)) == -1 ) {
+               if( (select_ret=select( max_fd + 1, &read_set, NULL, NULL, NULL )) == -1 ) {
                        osrfLogWarning( OSRF_LOG_MARK, "Select returned error %d on check_children: %s",
-                                       errno, strerror( errno ) );
+                               errno, strerror( errno ));
                }
-               osrfLogInfo(OSRF_LOG_MARK,
-                               "select() completed after waiting on children to become available");
+               osrfLogInfo( OSRF_LOG_MARK,
+                       "select() completed after waiting on children to become available" );
 
        } else {
 
@@ -831,9 +879,9 @@ static void check_children( prefork_simple* forker, int forever ) {
                tv.tv_sec   = 0;
                tv.tv_usec  = 0;
 
-               if( (select_ret=select( max_fd + 1, &read_set, NULL, NULL, &tv)) == -1 ) {
+               if( (select_ret=select( max_fd + 1, &read_set, NULL, NULL, &tv )) == -1 ) {
                        osrfLogWarning( OSRF_LOG_MARK, "Select returned error %d on check_children: %s",
-                                       errno, strerror( errno ) );
+                               errno, strerror( errno ));
                }
        }
 
@@ -847,18 +895,18 @@ static void check_children( prefork_simple* forker, int forever ) {
        int num_handled = 0;
        do {
                next_child = cur_child->next;
-               if( FD_ISSET( cur_child->read_status_fd, &read_set ) ) {
+               if( FD_ISSET( cur_child->read_status_fd, &read_set )) {
                        osrfLogDebug( OSRF_LOG_MARK,
-                                       "Server received status from a child %d", cur_child->pid );
+                               "Server received status from a child %d", cur_child->pid );
 
                        num_handled++;
 
                        /* now suck off the data */
                        char buf[64];
-                       if( (n=read(cur_child->read_status_fd, buf, sizeof(buf) - 1)) < 0 ) {
+                       if( (n=read( cur_child->read_status_fd, buf, sizeof( buf ) - 1 )) < 0 ) {
                                osrfLogWarning( OSRF_LOG_MARK,
-                                               "Read error after select in child status read with errno %d: %s",
-                                               errno, strerror( errno ) );
+                                       "Read error after select in child status read with errno %d: %s",
+                                       errno, strerror( errno ));
                        }
                        else {
                                buf[n] = '\0';
@@ -908,22 +956,24 @@ static void prefork_child_wait( prefork_child* child ) {
 
                n = -1;
                int gotdata = 0;    // boolean; set to true if we get data
-               clr_fl(child->read_data_fd, O_NONBLOCK );
+               clr_fl( child->read_data_fd, O_NONBLOCK );
 
-               while( (n=read(child->read_data_fd, buf, READ_BUFSIZE-1)) > 0 ) {
+               // Read a request from the parent, via a pipe, into a growing_buffer.
+               while( (n = read( child->read_data_fd, buf, READ_BUFSIZE-1 )) > 0 ) {
                        buf[n] = '\0';
-                       osrfLogDebug(OSRF_LOG_MARK, "Prefork child read %d bytes of data", n);
-                       if(!gotdata) {
-                               set_fl(child->read_data_fd, O_NONBLOCK );
+                       osrfLogDebug( OSRF_LOG_MARK, "Prefork child read %d bytes of data", n );
+                       if( !gotdata ) {
+                               set_fl( child->read_data_fd, O_NONBLOCK );
                                gotdata = 1;
                        }
                        buffer_add( gbuf, buf );
                }
 
-               if( errno == EAGAIN ) n = 0;
+               if( errno == EAGAIN )
+                       n = 0;
 
                if( errno == EPIPE ) {
-                       osrfLogDebug(OSRF_LOG_MARK, "C child attempted read on broken pipe, exiting...");
+                       osrfLogDebug( OSRF_LOG_MARK, "C child attempted read on broken pipe, exiting..." );
                        break;
                }
 
@@ -931,40 +981,43 @@ static void prefork_child_wait( prefork_child* child ) {
 
                if( n < 0 ) {
                        osrfLogWarning( OSRF_LOG_MARK,
-                                       "Prefork child read returned error with errno %d", errno );
+                               "Prefork child read returned error with errno %d", errno );
                        break;
 
                } else if( gotdata ) {
+                       // Process the request
                        osrfLogDebug( OSRF_LOG_MARK, "Prefork child got a request.. processing.." );
                        terminate_now = prefork_child_process_request( child, gbuf->buf );
                        buffer_reset( gbuf );
                }
 
                if( terminate_now ) {
+                       // We're terminating prematurely -- presumably due to a fatal error condition.
                        osrfLogWarning( OSRF_LOG_MARK, "Prefork child terminating abruptly" );
                        break;
                }
 
                if( i < child->max_requests - 1 ) {
+                       // Report back to the parent for another request.
                        size_t msg_len = 9;
                        ssize_t len = write(
                                child->write_status_fd, "available" /*less than 64 bytes*/, msg_len );
                        if( len != msg_len ) {
-                               osrfLogError( OSRF_LOG_MARK, 
+                               osrfLogError( OSRF_LOG_MARK,
                                        "Drone terminating: unable to notify listener of availability: %s",
                                        strerror( errno ));
-                               buffer_free(gbuf);
-                               osrf_prefork_child_exit(child);
+                               buffer_free( gbuf );
+                               osrf_prefork_child_exit( child );
                        }
                }
        }
 
-       buffer_free(gbuf);
+       buffer_free( gbuf );
 
        osrfLogDebug( OSRF_LOG_MARK, "Child with max-requests=%d, num-served=%d exiting...[%ld]",
-                       child->max_requests, i, (long) getpid() );
+               child->max_requests, i, (long) getpid());
 
-       osrf_prefork_child_exit(child);
+       osrf_prefork_child_exit( child );
 }
 
 /**
@@ -991,12 +1044,21 @@ static void add_prefork_child( prefork_simple* forker, prefork_child* child ) {
        }
 }
 
+/**
+       @brief Delete and destroy a dead child from our list.
+       @param forker Pointer to the prefork_simple that owns the dead child.
+       @param pid Process ID of the dead child.
+
+       Look for the dead child first in the list of active children.  If you don't find it
+       there, look in the list of idle children.  If you find it, remove it from whichever
+       list it's on, and destroy it.
+*/
 static void del_prefork_child( prefork_simple* forker, pid_t pid ) {
 
        osrfLogDebug( OSRF_LOG_MARK, "Deleting Child: %d", pid );
 
        prefork_child* cur_child = NULL;
-       
+
        // Look first in the active list
        if( forker->first_child ) {
                cur_child = forker->first_child; /* current pointer */
@@ -1067,7 +1129,7 @@ static prefork_child* prefork_child_init( prefork_simple* forker,
                child = forker->free_list;
                forker->free_list = child->next;
        } else
-               child = safe_malloc(sizeof(prefork_child));
+               child = safe_malloc( sizeof( prefork_child ));
 
        child->pid              = 0;
        child->read_data_fd     = read_data_fd;
@@ -1126,11 +1188,11 @@ static void prefork_clear( prefork_simple* prefork ) {
        // don't become zombies.  We don't wait indefinitely, so it's possible that some
        // children will survive a bit longer.
        sleep( 1 );
-       while( (waitpid(-1, NULL, WNOHANG)) > 0) {
+       while( (waitpid( -1, NULL, WNOHANG )) > 0 ) {
                --prefork->current_num_children;
        }
 
-       free(prefork->appname);
+       free( prefork->appname );
        prefork->appname = NULL;
 }