Cleanup of previous revision. Parse argv only once, building an array

of services we were passed on the command line.  Makes "dcm -f" (with no
service args) work as a side effect; it's unclear if this is a good thing.

Add comments that mention possibility of race conditions.

diff --git a/dcm/dcm.pc b/dcm/dcm.pc
index 2f583fe32ad47ca15d550ebb2b9ec3097e9853f5..8c7199aa2f1b7f73d0bb8775a43559c7281f767a 100644 (file)
--- a/dcm/dcm.pc
+++ b/dcm/dcm.pc
@@ -53,8 +53,8 @@ int main(int argc, char **argv)
   int enable;
   EXEC SQL END DECLARE SECTION;
   struct save_queue *sq;
-  int status;
-  char **arg = argv;
+  int status, srvcnt = 0;
+  char **arg = argv, *services[BUFSIZ];
 
   if (strchr(argv[0], '/'))
     strcpy(whoami, strrchr(argv[0], '/') + 1);
@@ -79,17 +79,32 @@ int main(int argc, char **argv)
              exit(1);
            }
        }
+      else
+       /* Doesn't begin with a dash, is a service name.
+        * Build an array of them we can iterate through later.
+        */
+       {
+         services[srvcnt] = malloc(SERVERS_NAME_SIZE);
+         if (!services[srvcnt])
+           {
+             com_err(whoami, 0, "Out of memory!");
+             exit(1);
+           }
+         strncpy(services[srvcnt], *arg, SERVERS_NAME_SIZE);
+         srvcnt++;
+       }
     }
 
-  /* if services were specified on the command line, do just those ones */
-  if (argc > 1)
+  /* Iterate through services specified on the command line, if any. */
+  if (srvcnt > 0)
     {
-      for (i = 1; i < argc; i++)
+      for (i = 0; i < srvcnt; i++)
        {
-         if (argv[i][0] == '-')
-           continue;
-         if (generate_service(argv[i], force))
-           do_hosts(argv[i]);
+         if (generate_service(services[i], force))
+           {
+             do_hosts(services[i]);
+             free(services[i]);
+           }
        }
       exit(0);
     }
@@ -184,6 +199,10 @@ int generate_service(char *name, int force)
   /* Someone might try to run a DCM from the command line while the
    * regular one is running, which will bypass the "interval" test.
    * Check inprogress to make sure they don't stomp on themselves.
+   *
+   * Note that there is still a race condition here, and this doesn't
+   * absolutely prevent 2 DCMs from stepping on one another, but it
+   * does reduce the window of vulnerability greatly.
    */
   if (inprogress == 1)
     {
@@ -305,6 +324,9 @@ void do_hosts(char *service)
   strtrim(target);
   strtrim(script);
 
+  /* Rudimentary locking.  Doesn't eliminate the possibility of 2 DCMs
+   * stepping on one another, but makes it harder.
+   */
   if (inprogress == 1)
     {
       com_err(whoami, 0, "DCM for service `%s' already in progress", name);
@@ -337,6 +359,8 @@ void do_hosts(char *service)
 
       EXEC SQL SELECT inprogress INTO :inprogress FROM serverhosts
        WHERE service = UPPER(:service) AND mach_id = :mid;
+      /* Check if someone got here before we did.
+       * There's still a race condition here, but it's a small one. */
       if (inprogress == 1)
        {
          com_err(whoami, 0, "DCM for service `%s' to host `%s' already in progress", service, name);