ZOOKEEPER-3599: cli.c: Resuscitate "old-style" argument parsing

ZOOKEEPER-2122 added SSL support to the C client and to the `cli_st`/`mt` tools.  It introduced (much-welcome!) GNU-style `getopt_long` argument parsing, but did not try to preserve backwards compatibility.

An earlier version of this https://github.com/apache/zookeeper/pull/1131 introduced (optional) POSIX-style `getopt` argument parsing, and was consequently largely obsoleted by the SSL update.  It did, however, support "old-style" arguments to avoid [breaking workflows](https://xkcd.com/1172/).

This patch salvages that feature while preserving the `getopt_long` goodness.  It is "legacy-only"; adding new positional arguments is not supported—as discussed in this thread: https://github.com/apache/zookeeper/pull/1131#pullrequestreview-320870245.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>

Closes #1131 from ztzg/ZOOKEEPER-3599-use-getopt-in-cli-c

1 files changed, 50 insertions, 31 deletions

diff --git a/zookeeper-client/zookeeper-client-c/src/cli.c b/zookeeper-client/zookeeper-client-c/src/cli.c
index 4a9c76fbc..64b328c4e 100644
--- a/zookeeper-client/zookeeper-client-c/src/cli.c
+++ b/zookeeper-client/zookeeper-client-c/src/cli.c
@@ -58,8 +58,8 @@ static zhandle_t *zh;
 static clientid_t myid;
 static const char *clientIdFile = 0;
 struct timeval startTime;
-static char *cmd;
-static char *cert;
+static const char *cmd;
+static const char *cert;
 static int batchMode=0;
 
 static int to_send=0;
@@ -330,7 +330,7 @@ int startsWith(const char *line, const char *prefix) {
 static const char *hostPort;
 static int verbose = 0;
 
-void processline(char *line) {
+void processline(const char *line) {
     int rc;
     int async = ((line[0] == 'a') && !(startsWith(line, "addauth ")));
     if (async) {
@@ -545,6 +545,7 @@ void processline(char *line) {
             int sequential = 0;
             int container = 0;
             int ttl = 0;
+            char *p = NULL;
 
             line++;
 
@@ -571,7 +572,7 @@ void processline(char *line) {
 
                     line++;
 
-                    ttl_value = strtol(line, &line, 10);
+                    ttl_value = strtol(line, &p, 10);
 
                     if (ttl_value <= 0) {
                         fprintf(stderr, "ttl value must be a positive integer\n");
@@ -579,7 +580,7 @@ void processline(char *line) {
                     }
 
                     // move back line pointer to the last digit
-                    line--;
+                    line = p - 1;
 
                     break;
                 default:
@@ -713,15 +714,15 @@ void processline(char *line) {
       zoo_add_auth(zh, line, ptr, ptr ? strlen(ptr) : 0, NULL, NULL);
     }
 }
+
 /*
- * Look for a command in the form 'cmd:command'.
- * Strips the prefix and copies the command in buf.
+ * Look for a command in the form 'cmd:command', and store a pointer
+ * to the command (without its prefix) into *buf if found.
+ *
  * Returns 0 if the argument does not start with the prefix.
- * Returns -1 in case of error (command too long).
  * Returns 1 in case of success.
- * 
  */
-int handleBatchMode(char* arg, char* buf, size_t maxlen) {    
+int handleBatchMode(const char* arg, const char** buf) {
     size_t cmdlen = strlen(arg);
     if (cmdlen < 4) {
         // too short
@@ -731,15 +732,7 @@ int handleBatchMode(char* arg, char* buf, size_t maxlen) {
     if(strncmp("cmd:", arg, 4) != 0){
         return 0;        
     }
-    // we must leave space for the NULL terminator
-    if (cmdlen >= maxlen) {
-          fprintf(stderr,
-                  "Command length %zu exceeds max length of %zu\n",
-                  cmdlen,
-                  maxlen);
-          return -1;
-    }
-    memcpy(cmd, arg + 4, cmdlen);
+    *buf = arg + 4;
     return 1;
 }
 
@@ -777,28 +770,21 @@ int main(int argc, char **argv) {
     while ((opt = getopt_long(argc, argv, "h:s:m:c:rd", long_options, &option_index)) != -1) {
         switch (opt) {
             case 'h':
-                hostPort = strdup(optarg);
+                hostPort = optarg;
                 break;
             case 'm':
-                clientIdFile = strdup(optarg);
-                fh = fopen(clientIdFile, "r");
-                if (fh) {
-                    if (fread(&myid, sizeof(myid), 1, fh) != sizeof(myid)) {
-                        memset(&myid, 0, sizeof(myid));
-                    }
-                    fclose(fh);
-                }
+                clientIdFile = optarg;
                 break;
             case 'r':
                 flags = ZOO_READONLY;
                 break;
             case 'c':
-                cmd = strdup(optarg);
+                cmd = optarg;
                 batchMode = 1;
                 fprintf(stderr,"Batch mode: %s\n",cmd);
                 break;
             case 's':
-                cert = strdup(optarg);
+                cert = optarg;
                 break;
             case 'd':
                 verbose = 1;
@@ -819,7 +805,30 @@ int main(int argc, char **argv) {
         }
     }
 
-    if (!hostPort) {
+    if (!hostPort && optind < argc) {
+        /*
+         * getopt_long did not find a '-h <connect-string>' option.
+         *
+         * The invoker may be using using the "old-style" command
+         * syntax, with positional parameters and "magical" prefixes
+         * such as 'cmd:'; let's see if we can make sense of it.
+         */
+        hostPort = argv[optind++];
+
+        if (optind < argc && !cmd && !clientIdFile) {
+            int batchModeRes = handleBatchMode(argv[optind], &cmd);
+            if (batchModeRes == 1) {
+                batchMode=1;
+                fprintf(stderr, "Batch mode: '%s'\n", cmd);
+            } else {
+                clientIdFile = argv[optind];
+            }
+
+            optind++;
+        }
+    }
+
+    if (!hostPort || optind < argc) {
         fprintf(stderr,
                 "\nUSAGE:    %s -h zk_host_1:port_1,zk_host_2:port_2,... [OPTIONAL ARGS]\n\n"
                 "MANDATORY ARGS:\n"
@@ -839,6 +848,16 @@ int main(int argc, char **argv) {
         return 2;
     }
 
+    if (clientIdFile) {
+        fh = fopen(clientIdFile, "r");
+        if (fh) {
+            if (fread(&myid, sizeof(myid), 1, fh) != sizeof(myid)) {
+                memset(&myid, 0, sizeof(myid));
+            }
+            fclose(fh);
+        }
+    }
+
 #ifdef YCA
     strcpy(appId,"yahoo.example.yca_test");
     cert = yca_get_cert_once(appId);