Com: separate iocsh argument splitting

modules/libcom/src/iocsh/iocsh.cpp

@@ -13,6 +13,9 @@
 /* Adapted to C++ by Eric Norum   Date: 18DEC2000 */
 
 #include <exception>
+#include <vector>
+#include <map>
+#include <string>
 
 #include <stddef.h>
 #include <string.h>
@@ -85,14 +88,36 @@ static epicsThreadPrivateId iocshContextId;
 /*
  * I/O redirection
  */
-#define NREDIRECTS   5
+namespace {
 struct iocshRedirect {
+    // pointer within the current line buffer
     const char *name;
+    // a static string constant: "a", "r", or "w"
     const char *mode;
     FILE       *fp;
     FILE       *oldFp;
-    int         mustRestore;
+    bool        mustRestore;
+
+    iocshRedirect()
+        :name(NULL)
+        ,mode(NULL)
+        ,fp(NULL)
+        ,oldFp(NULL)
+        ,mustRestore(false)
+    {}
+
+    ~iocshRedirect() {
+        close();
+    }
+
+    void close() {
+        if(fp) {
+            (void)fclose(fp);
+            fp = NULL;
+        }
+    }
 };
+} // namespace
 
 /*
  * Set up module variables
@@ -178,7 +203,255 @@ const iocshCmdDef * epicsStdCall iocshFindCommand(const char *name)
     return (iocshCmdDef *) registryFind(iocshCmdID, name);
 }
 
+static void
+showError (const char *filename, int lineno, const char *msg, ...)
+{
+    va_list ap;
+
+    va_start (ap, msg);
+    if (filename)
+        fprintf(epicsGetStderr(), "%s line %d: ", filename, lineno);
+    vfprintf (epicsGetStderr(), msg, ap);
+    fputc ('\n', epicsGetStderr());
+    va_end (ap);
+}
+
 namespace {
+
+struct Tokenize {
+    // holds pointers to the most recently split() line buffer
+    std::vector<const char*> argv;
+    typedef  std::map<int, iocshRedirect> redirects_t;
+    redirects_t redirects;
+    iocshRedirect* redirect;
+    bool noise;
+
+    Tokenize()
+        :redirect(NULL)
+        ,noise(true)
+    {}
+
+    ~Tokenize()
+    {
+        stopRedirect();
+    }
+
+    // if split()==true then argv.size()>=1 where
+    // argv[size()-1] is always a NULL.
+    size_t size() const {
+        return argv.empty() ? 0u : argv.size()-1u;
+    }
+    bool empty() const {
+        return argv.size()<=1u;
+    }
+
+    bool is_redirected(int fd) const {
+        redirects_t::const_iterator it = redirects.find(fd);
+        return it!=redirects.end() && !!it->second.name;
+    }
+
+    /* Break line into words.
+     * Stores pointers to line buffer this->argv and this->redirects[].name
+     */
+    bool split(const char *filename, int lineno, char *line, int& icin) {
+
+        argv.clear();
+        redirects.clear();
+        redirect = NULL;
+
+        int icout = 0;
+        bool inword = false;
+        bool backslash = false;
+        char quote = EOF;
+
+        for (;;) {
+            char c = line[icin++];
+            if (c == '\0')
+                break;
+
+            bool sep = (quote == EOF) && !backslash && (strchr (" \t(),\r", c));
+
+            if ((quote == EOF) && !backslash) {
+                int redirectFd = 1;
+                if (c == '\\') {
+                    backslash = true;
+                    continue;
+                }
+                if (c == '<') {
+                    if (redirect != NULL) {
+                        break;
+                    }
+                    redirect = &redirects[0];
+                    sep = 1;
+                    redirect->mode = "r";
+                }
+                if ((c >= '1') && (c <= '9') && (line[icin] == '>')) {
+                    redirectFd = c - '0';
+                    c = '>';
+                    icin++;
+                }
+                if (c == '>') {
+                    if (redirect != NULL)
+                        break;
+                    redirect = &redirects[redirectFd];
+                    sep = 1;
+                    if (line[icin] == '>') {
+                        icin++;
+                        redirect->mode = "a";
+                    }
+                    else {
+                        redirect->mode = "w";
+                    }
+                }
+            }
+            if (inword) {
+                if (c == quote) {
+                    quote = EOF;
+                }
+                else {
+                    if ((quote == EOF) && !backslash) {
+                        if (sep) {
+                            inword = false;
+                            // this "closes" a sub-string which was previously
+                            // stored in argv[] or redirects[].name
+                            line[icout++] = '\0';
+                        }
+                        else if ((c == '"') || (c == '\'')) {
+                            quote = c;
+                        }
+                        else {
+                            line[icout++] = c;
+                        }
+                    }
+                    else {
+                        line[icout++] = c;
+                    }
+                }
+            }
+            else {
+                if (!sep) {
+                    if (((c == '"') || (c == '\'')) && !backslash) {
+                        quote = c;
+                    }
+                    if (redirect != NULL) {
+                        if (redirect->name) { // double redirect?
+                            return false;
+                        }
+                        redirect->name = line + icout;
+                        redirect = NULL;
+                    }
+                    else {
+                        argv.push_back(line + icout);
+                    }
+                    if (quote == EOF)
+                        line[icout++] = c;
+                    inword = true;
+                }
+            }
+            backslash = false;
+        }
+
+        if (inword)
+            line[icout++] = '\0';
+
+
+        if (redirect != NULL) {
+            if(noise)
+                showError(filename, lineno, "Illegal redirection.");
+            return true;
+        }
+        if (quote != EOF) {
+            if(noise)
+                showError(filename, lineno, "Unbalanced quote.");
+            return true;
+        }
+        if (backslash) {
+            if(noise)
+                showError(filename, lineno, "Trailing backslash.");
+            return true;
+        }
+
+        argv.push_back(NULL);
+
+        return false;
+    }
+
+    int openRedirect(const char *filename, int lineno)
+    {
+        for(redirects_t::iterator it = redirects.begin(), end = redirects.end();
+            it!=end; ++it)
+        {
+            iocshRedirect *redirect = &it->second;
+            if(!redirect->name)
+                continue;
+
+            redirect->fp = fopen(redirect->name, redirect->mode);
+            if (redirect->fp == NULL) {
+                int err = errno;
+                showError(filename, lineno, "Can't open \"%s\": %s.",
+                                            redirect->name, strerror(err));
+                redirect->name = NULL;
+                // caller will clear tok.redirects
+                return -1;
+            }
+            redirect->mustRestore = false;
+        }
+        return 0;
+    }
+
+    void startRedirect()
+    {
+        for(redirects_t::iterator it = redirects.begin(), end = redirects.end();
+            it!=end; ++it)
+        {
+            iocshRedirect *redirect = &it->second;
+            if(!redirect->fp)
+                continue;
+
+            switch(it->first) {
+            case 0:
+                redirect->oldFp = epicsGetThreadStdin();
+                epicsSetThreadStdin(redirect->fp);
+                redirect->mustRestore = true;
+                break;
+            case 1:
+                redirect->oldFp = epicsGetThreadStdout();
+                epicsSetThreadStdout(redirect->fp);
+                redirect->mustRestore = true;
+                break;
+            case 2:
+                redirect->oldFp = epicsGetThreadStderr();
+                epicsSetThreadStderr(redirect->fp);
+                redirect->mustRestore = true;
+                break;
+            }
+        }
+    }
+
+    void stopRedirect()
+    {
+        for(redirects_t::iterator it = redirects.begin(), end = redirects.end();
+            it!=end; ++it)
+        {
+            iocshRedirect *redirect = &it->second;
+            if(!redirect->fp)
+                continue;
+
+            if (redirect->mustRestore) {
+                switch(it->first) {
+                case 0: epicsSetThreadStdin(redirect->oldFp);  break;
+                case 1: epicsSetThreadStdout(redirect->oldFp); break;
+                case 2: epicsSetThreadStderr(redirect->oldFp); break;
+                }
+                redirect->mustRestore = false;
+            }
+
+            redirect->close();
+            redirect->name = NULL;
+        }
+    }
+};
+
 #ifdef USE_READLINE
 
 char* iocsh_complete_command(const char* word, int notfirst)
@@ -384,22 +657,6 @@ void epicsStdCall iocshFree(void)
     iocshTableUnlock ();
 }
 
-/*
- * Report an error
- */
-static void
-showError (const char *filename, int lineno, const char *msg, ...)
-{
-    va_list ap;
-
-    va_start (ap, msg);
-    if (filename)
-        fprintf(epicsGetStderr(), "%s line %d: ", filename, lineno);
-    vfprintf (epicsGetStderr(), msg, ap);
-    fputc ('\n', epicsGetStderr());
-    va_end (ap);
-}
-
 static int
 cvtArg (const char *filename, int lineno, char *arg, iocshArgBuf *argBuf,
     const iocshArg *piocshArg)
@@ -481,95 +738,6 @@ cvtArg (const char *filename, int lineno, char *arg, iocshArgBuf *argBuf,
     return 1;
 }
 
-/*
- * Open redirected I/O
- */
-static int
-openRedirect(const char *filename, int lineno, struct iocshRedirect *redirect)
-{
-    int i;
-
-    for (i = 0 ; i < NREDIRECTS ; i++, redirect++) {
-        if (redirect->name != NULL) {
-            redirect->fp = fopen(redirect->name, redirect->mode);
-            if (redirect->fp == NULL) {
-                showError(filename, lineno, "Can't open \"%s\": %s.",
-                                            redirect->name, strerror(errno));
-                redirect->name = NULL;
-                while (i--) {
-                    redirect--;
-                    if (redirect->fp) {
-                        fclose(redirect->fp);
-                        redirect->fp = NULL;
-                    }
-                    redirect->name = NULL;
-                }
-                return -1;
-            }
-            redirect->mustRestore = 0;
-        }
-    }
-    return 0;
-}
-
-/*
- * Start I/O redirection
- */
-static void
-startRedirect(const char * /*filename*/, int /*lineno*/,
-              struct iocshRedirect *redirect)
-{
-    int i;
-
-    for (i = 0 ; i < NREDIRECTS ; i++, redirect++) {
-        if (redirect->fp != NULL) {
-            switch(i) {
-            case 0:
-                redirect->oldFp = epicsGetThreadStdin();
-                epicsSetThreadStdin(redirect->fp);
-                redirect->mustRestore = 1;
-                break;
-            case 1:
-                redirect->oldFp = epicsGetThreadStdout();
-                epicsSetThreadStdout(redirect->fp);
-                redirect->mustRestore = 1;
-                break;
-            case 2:
-                redirect->oldFp = epicsGetThreadStderr();
-                epicsSetThreadStderr(redirect->fp);
-                redirect->mustRestore = 1;
-                break;
-            }
-        }
-    }
-}
-
-/*
- * Finish up I/O redirection
- */
-static void
-stopRedirect(const char *filename, int lineno, struct iocshRedirect *redirect)
-{
-    int i;
-
-    for (i = 0 ; i < NREDIRECTS ; i++, redirect++) {
-        if (redirect->fp != NULL) {
-            if (fclose(redirect->fp) != 0)
-                showError(filename, lineno, "Error closing \"%s\": %s.",
-                                            redirect->name, strerror(errno));
-            redirect->fp = NULL;
-            if (redirect->mustRestore) {
-                switch(i) {
-                case 0: epicsSetThreadStdin(redirect->oldFp);  break;
-                case 1: epicsSetThreadStdout(redirect->oldFp); break;
-                case 2: epicsSetThreadStderr(redirect->oldFp); break;
-                }
-            }
-        }
-        redirect->name = NULL;
-    }
-}
-
 /*
  * "help" command
  */
@@ -692,22 +860,13 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
 {
     FILE *fp = NULL;
     const char *filename = NULL;
-    int icin, icout;
+    int icin;
     char c;
-    int quote, inword, backslash;
     const char *raw = NULL;;
     char *line = NULL;
     int lineno = 0;
-    int argc;
-    char **argv = NULL;
-    int argvCapacity = 0;
-    struct iocshRedirect *redirects = NULL;
-    struct iocshRedirect *redirect = NULL;
-    int sep;
     const char *prompt = NULL;
-    const char *ifs = " \t(),\r";
+    std::vector<iocshArgBuf> argBuf;
-    iocshArgBuf *argBuf = NULL;
-    int argBufCapacity = 0;
     struct iocshCommand *found;
     ReadlineContext readline;
     int wasOkToBlock;
@@ -716,6 +875,7 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
     iocshContext *context;
     char ** defines = NULL;
     int ret = 0;
+    Tokenize tokenize;
 
     iocshInit();
 
@@ -758,15 +918,6 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
         scope.onerr = Break;
     }
 
-    /*
-     * Set up redirection
-     */
-    redirects = (struct iocshRedirect *)calloc(NREDIRECTS, sizeof *redirects);
-    if (redirects == NULL) {
-        fprintf(epicsGetStderr(), "Out of memory!\n");
-        return -1;
-    }
-
     /*
      * Parse macro definitions, this check occurs before creating the
      * macro handle to simplify cleanup.
@@ -774,7 +925,6 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
 
     if (macros) {
         if (macParseDefns(NULL, macros, &defines) < 0) {
-            free(redirects);
             return -1;
         }
     }
@@ -786,7 +936,6 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
         context = (iocshContext*)calloc(1, sizeof(*context));
         if (!context || macCreateHandle(&context->handle, pairs)) {
             errlogMessage("iocsh: macCreateHandle failed.");
-            free(redirects);
             free(context);
             return -1;
         }
@@ -900,181 +1049,62 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
         /*
          * Break line into words
          */
-        icout = 0;
+        if (tokenize.split(filename, lineno, line, icin)) {
-        inword = 0;
-        argc = 0;
-        quote = EOF;
-        backslash = 0;
-        redirect = NULL;
-        for (;;) {
-            if (argc >= argvCapacity) {
-                int newCapacity = argvCapacity + 20;
-                char **newv = (char **)realloc (argv, newCapacity * sizeof *argv);
-                if (newv == NULL) {
-                    fprintf (epicsGetStderr(), "Out of memory!\n");
-                    argc = -1;
-                    scope.errored = true;
-                    break;
-                }
-                argv = newv;
-                argvCapacity = newCapacity;
-            }
-            c = line[icin++];
-            if (c == '\0')
-                break;
-            if ((quote == EOF) && !backslash && (strchr (ifs, c)))
-                sep = 1;
-            else
-                sep = 0;
-            if ((quote == EOF) && !backslash) {
-                int redirectFd = 1;
-                if (c == '\\') {
-                    backslash = 1;
-                    continue;
-                }
-                if (c == '<') {
-                    if (redirect != NULL) {
-                        break;
-                    }
-                    redirect = &redirects[0];
-                    sep = 1;
-                    redirect->mode = "r";
-                }
-                if ((c >= '1') && (c <= '9') && (line[icin] == '>')) {
-                    redirectFd = c - '0';
-                    c = '>';
-                    icin++;
-                }
-                if (c == '>') {
-                    if (redirect != NULL)
-                        break;
-                    if (redirectFd >= NREDIRECTS) {
-                        redirect = &redirects[1];
-                        break;
-                    }
-                    redirect = &redirects[redirectFd];
-                    sep = 1;
-                    if (line[icin] == '>') {
-                        icin++;
-                        redirect->mode = "a";
-                    }
-                    else {
-                        redirect->mode = "w";
-                    }
-                }
-            }
-            if (inword) {
-                if (c == quote) {
-                    quote = EOF;
-                }
-                else {
-                    if ((quote == EOF) && !backslash) {
-                        if (sep) {
-                            inword = 0;
-                            line[icout++] = '\0';
-                        }
-                        else if ((c == '"') || (c == '\'')) {
-                            quote = c;
-                        }
-                        else {
-                            line[icout++] = c;
-                        }
-                    }
-                    else {
-                        line[icout++] = c;
-                    }
-                }
-            }
-            else {
-                if (!sep) {
-                    if (((c == '"') || (c == '\'')) && !backslash) {
-                        quote = c;
-                    }
-                    if (redirect != NULL) {
-                        if (redirect->name != NULL) {
-                            argc = -1;
-                            break;
-                        }
-                        redirect->name = line + icout;
-                        redirect = NULL;
-                    }
-                    else {
-                        argv[argc++] = line + icout;
-                    }
-                    if (quote == EOF)
-                        line[icout++] = c;
-                    inword = 1;
-                }
-            }
-            backslash = 0;
-        }
-        if (redirect != NULL) {
-            showError(filename, lineno, "Illegal redirection.");
             scope.errored = true;
             continue;
-        }
+
-        if (argc < 0) {
+        } else if (tokenize.empty() && tokenize.redirects.empty()) {
             break;
         }
-        if (quote != EOF) {
-            showError(filename, lineno, "Unbalanced quote.");
-            scope.errored = true;
-            continue;
-        }
-        if (backslash) {
-            showError(filename, lineno, "Trailing backslash.");
-            scope.errored = true;
-            continue;
-        }
-        if (inword)
-            line[icout++] = '\0';
-        argv[argc] = NULL;
 
         /*
          * Special case -- Redirected input but no command
          * Treat as if 'iocsh filename'.
          */
-        if ((argc == 0) && (redirects[0].name != NULL)) {
+        if (tokenize.empty() && tokenize.is_redirected(0)) {
-            const char *commandFile = redirects[0].name;
+            const char* commandFile = tokenize.redirects[0].name;
-            redirects[0].name = NULL;
+            tokenize.redirects[0].name = NULL;
-            if (openRedirect(filename, lineno, redirects) < 0)
+            if (tokenize.openRedirect(filename, lineno) < 0)
                 continue;
-            startRedirect(filename, lineno, redirects);
+            tokenize.startRedirect();
             if(iocshBody(commandFile, NULL, macros))
                 scope.errored = true;
-            stopRedirect(filename, lineno, redirects);
+            tokenize.stopRedirect();
             continue;
         }
 
         /*
          * Special command?
          */
-        if ((argc > 0) && (strcmp(argv[0], "exit") == 0))
+        if (!tokenize.empty() && (strcmp(tokenize.argv[0], "exit") == 0))
             break;
 
         /*
          * Set up redirection
          */
-        if ((openRedirect(filename, lineno, redirects) == 0) && (argc > 0)) {
+        if ((tokenize.openRedirect(filename, lineno) == 0) && !tokenize.empty()) {
             // error unless a function is actually called.
             // handles command not-found and arg parsing errors.
             scope.errored = true;
             /*
              * Look up command
              */
-            found = (iocshCommand *)registryFind (iocshCmdID, argv[0]);
+            found = (iocshCommand *)registryFind (iocshCmdID, tokenize.argv[0]);
             if (found) {
                 /*
                  * Process arguments and call function
                  */
                 struct iocshFuncDef const *piocshFuncDef = found->def.pFuncDef;
-                for (int iarg = 0 ; ; ) {
+                // argBuf resize() with one extra so that &argBuf[0] doesn't trigger
-                    if (iarg == piocshFuncDef->nargs) {
+                // windows debug assert.
-                        startRedirect(filename, lineno, redirects);
+                argBuf.resize(size_t(piocshFuncDef->nargs)+1u);
+                for (size_t iarg = 0 ; ; ) {
+                    if (iarg == size_t(piocshFuncDef->nargs)) {
+                        tokenize.startRedirect();
                         /* execute */
                         scope.errored = false;
                         try {
-                            (*found->def.func)(argBuf);
+                            (*found->def.func)(&argBuf[0]);
                         } catch(std::exception& e){
                             fprintf(epicsGetStderr(), "c++ error: %s\n", e.what());
                             scope.errored = true;
@@ -1084,40 +1114,30 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
                         }
                         break;
                     }
-                    if (iarg >= argBufCapacity) {
-                        int newCapacity = argBufCapacity + 20;
-                        void *newBuf = realloc(argBuf, newCapacity * sizeof *argBuf);
-                        if (newBuf == NULL) {
-                            fprintf (epicsGetStderr(), "Out of memory!\n");
-                            break;
-                        }
-                        argBuf = (iocshArgBuf *) newBuf;
-                        argBufCapacity = newCapacity;
-                    }
                     if (piocshFuncDef->arg[iarg]->type == iocshArgArgv) {
-                        argBuf[iarg].aval.ac = argc-iarg;
+                        argBuf[iarg].aval.ac = tokenize.size()-iarg;
-                        argBuf[iarg].aval.av = argv+iarg;
+                        argBuf[iarg].aval.av = const_cast<char**>(&tokenize.argv[iarg]);
                         iarg = piocshFuncDef->nargs;
                     }
                     else {
                         if (!cvtArg (filename, lineno,
-                                ((iarg < argc) ? argv[iarg+1] : NULL),
+                                ((iarg < tokenize.size()) ? const_cast<char*>(tokenize.argv[iarg+1]) : NULL),
                                 &argBuf[iarg], piocshFuncDef->arg[iarg]))
                             break;
                         iarg++;
                     }
                 }
-                if ((prompt != NULL) && (strcmp(argv[0], "epicsEnvSet") == 0)) {
+                if ((prompt != NULL) && (strcmp(tokenize.argv[0], "epicsEnvSet") == 0)) {
                     const char *newPrompt;
                     if ((newPrompt = envGetConfigParamPtr(&IOCSH_PS1)) != NULL)
                         prompt = newPrompt;
                 }
             }
             else {
-                showError(filename, lineno, "Command %s not found.", argv[0]);
+                showError(filename, lineno, "Command %s not found.", tokenize.argv[0]);
             }
         }
-        stopRedirect(filename, lineno, redirects);
+        tokenize.stopRedirect();
     }
     macPopScope(handle);
 
@@ -1130,13 +1150,7 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
     }
     if (fp && (fp != stdin))
         fclose (fp);
-    if (redirects != NULL) {
-        stopRedirect(filename, lineno, redirects);
-        free (redirects);
-    }
     free(line);
-    free (argv);
-    free (argBuf);
     errlogFlush();
     epicsThreadSetOkToBlock(wasOkToBlock);
     return ret;

modules/libcom/test/iocshTest.cpp

@@ -14,6 +14,7 @@
 #include <dbDefs.h>
 #include <iocsh.h>
 #include <libComRegister.h>
+#include <dbmf.h>
 
 #include <epicsUnitTest.h>
 #include <testMain.h>
@@ -128,5 +129,7 @@ MAIN(iocshTest)
     testPosition("after_error_1", false);
     reached.clear();
 
+    // cleanup after macLib to avoid valgrind false positives
+    dbmfFreeChunks();
     return testDone();
 }

modules/libcom/test/iocshTestSuccess.cmd

@@ -3,6 +3,7 @@ help
 echo This works
 pwd
 epicsThreadSleep 0.0
+
 epicsThreadSleep 0
 epicsThreadSleep
 position success