improve command-line parsing

example invocations now flagged as invalid:
  specifying UTC offset with size-based rotation
  specifying -l with size-based rotation
  specifying both -l and UTC offset

range checking of integer parameters not attempted; basic data type issues may need
to be addressed first such as the use of unsigned int for max file size


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@596698 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 71 insertions, 46 deletions

diff --git a/support/rotatelogs.c b/support/rotatelogs.c
index c6f9904938..451ea13a76 100644
--- a/support/rotatelogs.c
+++ b/support/rotatelogs.c
@@ -42,6 +42,7 @@
 #include "apr_file_info.h"
 #include "apr_general.h"
 #include "apr_time.h"
+#include "apr_getopt.h"
 
 #if APR_HAVE_STDLIB_H
 #include <stdlib.h>
@@ -60,6 +61,36 @@
 #define MAX_PATH        1024
 #endif
 
+static void usage(const char *argv0, const char *reason)
+{
+    if (reason) {
+        fprintf(stderr, "%s\n", reason);
+    }
+    fprintf(stderr,
+            "Usage: %s [-l] <logfile> <rotation time in seconds> "
+            "[offset minutes from UTC] or <rotation size in megabytes>\n\n",
+            argv0);
+#ifdef OS2
+    fprintf(stderr,
+            "Add this:\n\nTransferLog \"|%s.exe /some/where 86400\"\n\n",
+            argv0);
+#else
+    fprintf(stderr,
+            "Add this:\n\nTransferLog \"|%s /some/where 86400\"\n\n",
+            argv0);
+    fprintf(stderr,
+            "or \n\nTransferLog \"|%s /some/where 5M\"\n\n", argv0);
+#endif
+    fprintf(stderr,
+            "to httpd.conf. The generated name will be /some/where.nnnn "
+            "where nnnn is the\nsystem time at which the log nominally "
+            "starts (N.B. if using a rotation time,\nthe time will always "
+            "be a multiple of the rotation time, so you can synchronize\n"
+            "cron scripts with it). At the end of each rotation time or "
+            "when the file size\nis reached a new log is started.\n");
+    exit(1);
+}
+
 int main (int argc, const char * const argv[])
 {
     char buf[BUFSIZE], buf2[MAX_PATH], errbuf[ERRMSGSZ];
@@ -73,69 +104,63 @@ int main (int argc, const char * const argv[])
     const char *szLogRoot;
     apr_file_t *f_stdin, *nLogFD = NULL, *nLogFDprev = NULL;
     apr_pool_t *pool;
+    apr_getopt_t *opt;
+    apr_status_t rv;
+    char c;
+    const char *optarg;
     char *ptr = NULL;
-    int argBase = 0;
-    int argFile = 1;
-    int argIntv = 2;
-    int argOffset = 3;
 
     apr_app_initialize(&argc, &argv, NULL);
     atexit(apr_terminate);
 
     apr_pool_create(&pool, NULL);
-    if ((argc > 2) && (strcmp(argv[1], "-l") == 0)) {
-        argBase++;
-        argFile += argBase;
-        argIntv += argBase;
-        argOffset += argBase;
-        use_localtime = 1;
+    apr_getopt_init(&opt, pool, argc, argv);
+    while ((rv = apr_getopt(opt, "l", &c, &optarg)) == APR_SUCCESS) {
+        switch (c) {
+        case 'l':
+            use_localtime = 1;
+            break;
+        }
     }
-    if (argc < (argBase + 3) || argc > (argBase + 4)) {
-        fprintf(stderr,
-                "Usage: %s [-l] <logfile> <rotation time in seconds> "
-                "[offset minutes from UTC] or <rotation size in megabytes>\n\n",
-                argv[0]);
-#ifdef OS2
-        fprintf(stderr,
-                "Add this:\n\nTransferLog \"|%s.exe /some/where 86400\"\n\n",
-                argv[0]);
-#else
-        fprintf(stderr,
-                "Add this:\n\nTransferLog \"|%s /some/where 86400\"\n\n",
-                argv[0]);
-        fprintf(stderr,
-                "or \n\nTransferLog \"|%s /some/where 5M\"\n\n", argv[0]);
-#endif
-        fprintf(stderr,
-                "to httpd.conf. The generated name will be /some/where.nnnn "
-                "where nnnn is the\nsystem time at which the log nominally "
-                "starts (N.B. if using a rotation time,\nthe time will always "
-                "be a multiple of the rotation time, so you can synchronize\n"
-                "cron scripts with it). At the end of each rotation time or "
-                "when the file size\nis reached a new log is started.\n");
-        exit(1);
+
+    if (rv != APR_EOF) {
+        usage(argv[0], NULL /* specific error message already issued */ );
+    }
+
+    if (opt->ind + 2 > argc) { /* must have at least a filename and a rotation parameter */
+        usage(argv[0], "Too few arguments");
     }
 
-    szLogRoot = argv[argFile];
+    szLogRoot = argv[opt->ind++];
 
-    ptr = strchr(argv[argIntv], 'M');
-    if (ptr) {
+    ptr = strchr(argv[opt->ind], 'M');
+    if (ptr) { /* rotation based on file size */
+        if (opt->ind + 1 != argc) {
+            usage(argv[0], "Wrong number of arguments for size-based rotation");
+        }
+        if (use_localtime) {
+            usage(argv[0], "-l is not supported with size-based rotation");
+        }
         if (*(ptr+1) == '\0') {
-            sRotation = atoi(argv[argIntv]) * 1048576;
+            sRotation = atoi(argv[opt->ind]) * 1048576;
         }
         if (sRotation == 0) {
-            fprintf(stderr, "Invalid rotation size parameter\n");
-            exit(1);
+            usage(argv[0], "Invalid rotation size parameter");
         }
     }
-    else {
-        if (argc >= (argBase + 4)) {
-            utc_offset = atoi(argv[argOffset]) * 60;
+    else { /* rotation based on elapsed time */
+        if (opt->ind + 1 != argc && opt->ind + 2 != argc) {
+            usage(argv[0], "Wrong number of arguments for time-based rotation");
+        }
+        if (opt->ind + 2 == argc) {
+            if (use_localtime) {
+                usage(argv[0], "UTC offset parameter is not valid with -l");
+            }
+            utc_offset = atoi(argv[opt->ind + 1]) * 60;
         }
-        tRotation = atoi(argv[argIntv]);
+        tRotation = atoi(argv[opt->ind]);
         if (tRotation <= 0) {
-            fprintf(stderr, "Rotation time must be > 0\n");
-            exit(6);
+            usage(argv[0], "Invalid rotation time parameter");
         }
     }