Patch from Scott McKellar:

1. I renamed __osrfRouter simply to router, and made it static.  We
had one global variable and one auto variable pointing to the same
object, causing some needless juggling.  Now we have just one
pointer.

2. I removed the leading underscores from __setupRouter().

3. I renamed the parameter to routerSignalHandler() from "signal"
to "signo", since "signal" is a reserved identifier.  I also added
some code to re-raise the signal caught.

[from a followup email]

> I chose instead to terminate the program by re-raising the signal.
> That way the parent process has a chance to detect that the program
> was terminated by a signal rather than by a normal return.

After posting this I realized that the router program runs as a
daemon, and its adopted parent process doesn't care whether it
terminates by a signal or by a normal return.  So there's not
much point in re-raising the signal.

It remains true that the signal handler should contrive to
terminate the program, either by exiting or by setting a flag that
the rest of the program tests.

[ The original patch, re-raising the signal, is applied. ]

4. In main() I moved two calls to free() so that they are reachable.

5. In main() I return either EXIT_SUCCESS or EXIT_FAILURE, which are
portable.  Otherwise we could find ourselves returning -1, which is
not portable.

6. In setupRouter() I arranged to free resource, and to free
tservers and tclients in the case of an early return.  I also free
router in the unlikely event that osrfRouterRun returns.

7. I reworded an error message to something that I think is more
clear.

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

diff --git a/src/router/osrf_router_main.c b/src/router/osrf_router_main.c
index a789d98..706d214 100644 (file)
--- a/src/router/osrf_router_main.c
+++ b/src/router/osrf_router_main.c
@@ -4,15 +4,22 @@
 #include "opensrf/log.h"
 #include <signal.h>
 
-osrfRouter* __osrfRouter = NULL;
+static osrfRouter* router = NULL;
 
-void routerSignalHandler( int signal ) {
-       osrfLogWarning( OSRF_LOG_MARK, "Received signal [%d], cleaning up...", signal );
+void routerSignalHandler( int signo ) {
+       osrfLogWarning( OSRF_LOG_MARK, "Received signal [%d], cleaning up...", signo );
        osrfConfigCleanup();
-       osrfRouterFree(__osrfRouter);
+       osrfRouterFree(router);
+       router = NULL;
+
+       // Exit by re-raising the signal so that the parent
+       // process can detect it
+       
+       signal( signo, SIG_DFL );
+       raise( signo );
 }
 
-static int __setupRouter( char* config, char* context );
+static int setupRouter( char* config, char* context );
 
 
 int main( int argc, char* argv[] ) {
@@ -27,13 +34,13 @@ int main( int argc, char* argv[] ) {
        init_proc_title( argc, argv );
        set_proc_title( "OpenSRF Router" );
 
-       return __setupRouter( config, context );
+       int rc = setupRouter( config, context );
        free(config);
        free(context);
-
+       return rc ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
-int __setupRouter( char* config, char* context ) {
+int setupRouter( char* config, char* context ) {
 
        osrfConfig* cfg = osrfConfigInit( config, context );
        osrfConfigSetDefaultConfig(cfg);
@@ -52,7 +59,7 @@ int __setupRouter( char* config, char* context ) {
        int llevel = 1;
        if(level) llevel = atoi(level);
 
-       if(!log_file) { fprintf(stderr, "Log file needed\n"); return -1; }
+       if(!log_file) { fprintf(stderr, "Log file name not specified\n"); return -1; }
 
        if(!strcmp(log_file, "syslog")) {
                osrfLogInit( OSRF_LOG_TYPE_SYSLOG, "router", llevel );
@@ -87,10 +94,12 @@ int __setupRouter( char* config, char* context ) {
 
        if( tclients->size == 0 || tservers->size == 0 ) {
                osrfLogError( OSRF_LOG_MARK, "We need trusted servers and trusted client to run the router...");
+               osrfStringArrayFree( tservers );
+               osrfStringArrayFree( tclients );
                return -1;
        }
 
-       osrfRouter* router = osrfNewRouter( server, 
+       router = osrfNewRouter( server,
                        username, resource, password, iport, tclients, tservers );
        
        signal(SIGHUP,routerSignalHandler);
@@ -99,16 +108,23 @@ int __setupRouter( char* config, char* context ) {
 
        if( (osrfRouterConnect(router)) != 0 ) {
                fprintf(stderr, "!!!! Unable to connect router to jabber server %s... exiting", server );
+               osrfRouterFree(router);
                return -1;
        }
 
        free(server); free(port); 
-       free(username); free(password); 
+       free(username); free(password);
+       free(resource);
 
-       __osrfRouter = router;
        daemonize();
        osrfRouterRun( router );
 
+       // Shouldn't get here, since osrfRouterRun()
+       // should go into an infinite loop
+
+       osrfRouterFree(router);
+       router = NULL;
+       
        return -1;
 }