From 60128ee9244e089efbbb15594a568b169ea8a704 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 20 Jun 2022 13:02:23 -0700 Subject: [PATCH] Com: separate iocsh argument splitting --- modules/libcom/src/iocsh/iocsh.cpp | 592 ++++++++++++----------- modules/libcom/test/iocshTest.cpp | 3 + modules/libcom/test/iocshTestSuccess.cmd | 1 + 3 files changed, 307 insertions(+), 289 deletions(-) diff --git a/modules/libcom/src/iocsh/iocsh.cpp b/modules/libcom/src/iocsh/iocsh.cpp index 8724166d3..5e8b39a1a 100644 --- a/modules/libcom/src/iocsh/iocsh.cpp +++ b/modules/libcom/src/iocsh/iocsh.cpp @@ -13,6 +13,9 @@ /* Adapted to C++ by Eric Norum Date: 18DEC2000 */ #include +#include +#include +#include #include #include @@ -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 argv; + typedef std::map 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"; - iocshArgBuf *argBuf = NULL; - int argBufCapacity = 0; + std::vector argBuf; 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; - 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."); + if (tokenize.split(filename, lineno, line, icin)) { 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)) { - const char *commandFile = redirects[0].name; - redirects[0].name = NULL; - if (openRedirect(filename, lineno, redirects) < 0) + if (tokenize.empty() && tokenize.is_redirected(0)) { + const char* commandFile = tokenize.redirects[0].name; + tokenize.redirects[0].name = NULL; + 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 ; ; ) { - if (iarg == piocshFuncDef->nargs) { - startRedirect(filename, lineno, redirects); + // argBuf resize() with one extra so that &argBuf[0] doesn't trigger + // windows debug assert. + 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.av = argv+iarg; + argBuf[iarg].aval.ac = tokenize.size()-iarg; + argBuf[iarg].aval.av = const_cast(&tokenize.argv[iarg]); iarg = piocshFuncDef->nargs; } else { if (!cvtArg (filename, lineno, - ((iarg < argc) ? argv[iarg+1] : NULL), + ((iarg < tokenize.size()) ? const_cast(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; diff --git a/modules/libcom/test/iocshTest.cpp b/modules/libcom/test/iocshTest.cpp index e07bf8686..7ca5c3867 100644 --- a/modules/libcom/test/iocshTest.cpp +++ b/modules/libcom/test/iocshTest.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -128,5 +129,7 @@ MAIN(iocshTest) testPosition("after_error_1", false); reached.clear(); + // cleanup after macLib to avoid valgrind false positives + dbmfFreeChunks(); return testDone(); } diff --git a/modules/libcom/test/iocshTestSuccess.cmd b/modules/libcom/test/iocshTestSuccess.cmd index 86c83f795..f6264137e 100644 --- a/modules/libcom/test/iocshTestSuccess.cmd +++ b/modules/libcom/test/iocshTestSuccess.cmd @@ -3,6 +3,7 @@ help echo This works pwd epicsThreadSleep 0.0 + epicsThreadSleep 0 epicsThreadSleep position success