From ea1f71377a14b35aaa85690226177f4dd8306617 Mon Sep 17 00:00:00 2001 From: Damien Diederen Date: Thu, 31 Oct 2019 08:54:21 +0100 Subject: [PATCH 1/3] ZOOKEEPER-3599: cli.c: Make processline's argument const There is no reason for it not to be--except for a silly const- correctness issue in 'strtol', and we can work around the latter by introducing a temporary variable. --- zookeeper-client/zookeeper-client-c/src/cli.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/src/cli.c b/zookeeper-client/zookeeper-client-c/src/cli.c index e8151ec8c66..4a0b5fd9b61 100644 --- a/zookeeper-client/zookeeper-client-c/src/cli.c +++ b/zookeeper-client/zookeeper-client-c/src/cli.c @@ -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: From 7b267b250f3ae49d1748e46518f1251ab30af71b Mon Sep 17 00:00:00 2001 From: Damien Diederen Date: Thu, 21 Nov 2019 18:10:10 +0100 Subject: [PATCH 2/3] ZOOKEEPER-3599: cli.c: Avoid duplicating 'optarg' The pointed-to string has the same lifetime as the argv[] entries, as per POSIX: https://pubs.opengroup.org/onlinepubs/009695399/functions/getopt.html https://stackoverflow.com/a/53508112 This avoids a tiny memory leak for options passed more than once :) --- zookeeper-client/zookeeper-client-c/src/cli.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/src/cli.c b/zookeeper-client/zookeeper-client-c/src/cli.c index 4a0b5fd9b61..332c3a2c735 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; @@ -778,10 +778,10 @@ 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); + clientIdFile = optarg; fh = fopen(clientIdFile, "r"); if (fh) { if (fread(&myid, sizeof(myid), 1, fh) != sizeof(myid)) { @@ -794,12 +794,12 @@ int main(int argc, char **argv) { 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; From b4e31065836a43d519dacf2de7ba726401dc1664 Mon Sep 17 00:00:00 2001 From: Damien Diederen Date: Thu, 21 Nov 2019 17:56:57 +0100 Subject: [PATCH 3/3] 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 a (much-welcome!) GNU-style 'getopt_long' argument parsing, but did not try to preserve backwards compatibility. An earlier version of https://github.com/apache/zookeeper/pull/1131 introduced optional POSIX-style 'getopt' argument parsing, and was consequently largely obsoleted by SSL update. It did, however, support "old-style" argument parsing, to avoid breaking workflows. 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 --- zookeeper-client/zookeeper-client-c/src/cli.c | 62 ++++++++++++------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/src/cli.c b/zookeeper-client/zookeeper-client-c/src/cli.c index 332c3a2c735..0c670feff42 100644 --- a/zookeeper-client/zookeeper-client-c/src/cli.c +++ b/zookeeper-client/zookeeper-client-c/src/cli.c @@ -714,15 +714,15 @@ void processline(const 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 @@ -732,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; } @@ -782,13 +774,6 @@ int main(int argc, char **argv) { break; case 'm': clientIdFile = optarg; - fh = fopen(clientIdFile, "r"); - if (fh) { - if (fread(&myid, sizeof(myid), 1, fh) != sizeof(myid)) { - memset(&myid, 0, sizeof(myid)); - } - fclose(fh); - } break; case 'r': flags = ZOO_READONLY; @@ -820,7 +805,30 @@ int main(int argc, char **argv) { } } - if (!hostPort) { + if (!hostPort && optind < argc) { + /* + * getopt_long did not find a '-h ' 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" @@ -842,6 +850,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);