pcs         98/05/09 08:26:31

  Modified:    src/main http_main.c
  Log:
  Start cleaning up the Win32 code. This cleans up the code which spawns
  child processes. The changes are
  
   -- remove an unbounded sprintf()
   -- always check for errors at all stages, and log an error message
   -- if an error occurs in creating a child, die, but make sure
      all already created children a killed correctly
   -- remove assert()'s
  
  Revision  Changes    Path
  1.345     +64 -13    apache-1.3/src/main/http_main.c
  
  Index: http_main.c
  ===================================================================
  RCS file: /export/home/cvs/apache-1.3/src/main/http_main.c,v
  retrieving revision 1.344
  retrieving revision 1.345
  diff -u -r1.344 -r1.345
  --- http_main.c       1998/05/09 13:23:56     1.344
  +++ http_main.c       1998/05/09 15:26:29     1.345
  @@ -4882,20 +4882,40 @@
       exit(0);
   }                            /* standalone_main */
   
  +/* Spawn a child Apache process. The child process has the command
  + * line arguments from argc and argv[], plus a -Z argument giving the
  + * name of an event. The child should open and poll or wait on this
  + * event. When it is signalled, the child should die.  prefix is a
  + * prefix string for the event name.
  + * 
  + * The child_num argument on entry contains a serial number for this
  + * child (used to create a unique event name). On exit, this number
  + * will have been incremented by one, ready for the next call.
  + *
  + * On exit, the value pointed to be *ev will contain the event created
  + * to signal the new child process.
  + *
  + * The return value is the handle to the child process if successful,
  + * else -1. If -1 is returned the error will already have been logged
  + * by ap_log_error(). 
  + */
   
   int create_event_and_spawn(int argc, char **argv, event **ev, int 
*child_num, char *prefix)
   {
  -    int pid = getpid();
       char buf[40], mod[200];
       int i, rv;
       char **pass_argv = (char **) alloca(sizeof(char *) * (argc + 3));
   
  -    sprintf(buf, "%s_%d", prefix, ++(*child_num));
  +    ap_snprintf(buf, sizeof(buf), "%s_%d", prefix, ++(*child_num));
       _flushall();
       *ev = CreateEvent(NULL, TRUE, FALSE, buf);
  +    if (!*ev) {
  +     ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL,
  +         "could not create event for child process");
  +     return -1;
  +    }
       APD2("create_event_and_spawn(): created process kill event %s", buf);
   
  -    ap_assert(*ev);
       pass_argv[0] = argv[0];
       pass_argv[1] = "-Z";
       pass_argv[2] = buf;
  @@ -4903,12 +4923,27 @@
        pass_argv[i + 2] = argv[i];
       }
       pass_argv[argc + 2] = NULL;
  -
   
  -    GetModuleFileName(NULL, mod, 200);
  +    rv = GetModuleFileName(NULL, mod, sizeof(mod));
  +    if (rv == sizeof(mod)) {
  +     /* mod[] was not big enough for our pathname */
  +     ap_log_error(APLOG_MARK, APLOG_ERR, NULL,
  +         "Internal error: path to Apache process too long");
  +     return -1;
  +    }
  +    if (rv == 0) {
  +     ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL,
  +         "GetModuleFileName() for current process");
  +     return -1;
  +    }
       rv = spawnv(_P_NOWAIT, mod, pass_argv);
  +    if (rv == -1) {
  +     ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL,
  +         "spawn of child process %s failed", mod);
  +     return -1;
  +    }
   
  -    return (rv);
  +    return rv;
   }
   
   /**********************************************************************
  @@ -4941,18 +4976,24 @@
       APD4("cleanup_processes: removed child in slot %d handle %d, max=%d", 
position, handle, *processes);
   }
   
  -void create_process(HANDLE *handles, HANDLE *events, int *processes, int 
*child_num, char *kill_event_name, int argc, char **argv)
  +int create_process(HANDLE *handles, HANDLE *events, int *processes, int 
*child_num, char *kill_event_name, int argc, char **argv)
   {
       int i = *processes;
       HANDLE kill_event;
  +    int child_handle;
   
  -    handles[i] = (HANDLE)create_event_and_spawn(argc, argv, &kill_event, 
child_num, kill_event_name);
  -    ap_assert(handles[i] >= 0);
  +    child_handle = create_event_and_spawn(argc, argv, &kill_event, 
child_num, kill_event_name);
  +    if (child_handle <= 0) {
  +     return -1;
  +    }
  +    handles[i] = (HANDLE)child_handle;
       events[i] = kill_event;
       (*processes)++;
   
       APD4("create_processes: created child in slot %d handle %d, max=%d", 
        (*processes)-1, handles[(*processes)-1], *processes);
  +
  +    return 0;
   }
   
   int master_main(int argc, char **argv)
  @@ -4970,7 +5011,7 @@
       HANDLE process_kill_events[MAX_PROCESSES];
       int current_live_processes = 0; /* number of child process we know about 
*/
       int processes_to_create = 0;    /* number of child processes to create */
  -    pool *pparent;  /* pool for the parent process. Cleaned on each restart 
*/
  +    pool *pparent = NULL;  /* pool for the parent process. Cleaned on each 
restart */
   
       nchild = 1;          /* only allowed one child process for current 
generation */
       processes_to_create = nchild;
  @@ -4989,9 +5030,13 @@
       start_mutex = ap_create_mutex(buf);
       ev = (event **) alloca(sizeof(event *) * nchild);
       child = (int *) alloca(sizeof(int) * (nchild+1));
  +
       while (processes_to_create--) {
        service_set_status(SERVICE_START_PENDING);
  -     create_process(process_handles, process_kill_events, 
&current_live_processes, &child_num, buf, argc, argv);
  +     if (create_process(process_handles, process_kill_events, 
  +         &current_live_processes, &child_num, buf, argc, argv) < 0) {
  +         goto die_now;
  +     }
       }
   
       service_set_status(SERVICE_RUNNING);
  @@ -5025,7 +5070,10 @@
                /* Shouldn't happen, but better safe than sorry */
                ap_log_error(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, server_conf,
                        "master_main: no child processes alive! creating one");
  -             create_process(process_handles, process_kill_events, 
&current_live_processes, &child_num, buf, argc, argv);
  +             if (create_process(process_handles, process_kill_events, 
  +                 &current_live_processes, &child_num, buf, argc, argv) < 0) {
  +                 goto die_now;
  +             }
                if (processes_to_create) {
                    processes_to_create--;
                }
  @@ -5103,6 +5151,7 @@
   
       APD2("*** main process shutdown, processes=%d ***", 
current_live_processes);
   
  +die_now:
       CloseHandle(signal_event);
   
       tmstart = time(NULL);
  @@ -5125,7 +5174,9 @@
       }
       service_set_status(SERVICE_STOPPED);
   
  -    ap_destroy_pool(pparent);
  +    if (pparent) {
  +     ap_destroy_pool(pparent);
  +    }
   
       ap_destroy_mutex(start_mutex);
       return (0);
  
  
  

Reply via email to