Experimental Physics and
Industrial Control System

Andrew Johnson via Core-talk <[email protected]> · Thu, 22 Aug 2019 23:14:41 -0000

Review: Needs Fixing

Still needs a little work (bug reported below), but getting close and looks much cleaner now. Needs Release Notes at least before merging.

Diff comments:

> diff --git a/modules/libcom/src/iocsh/iocsh.cpp b/modules/libcom/src/iocsh/iocsh.cpp
> index 0de90c8..7392bca 100644
> --- a/modules/libcom/src/iocsh/iocsh.cpp
> +++ b/modules/libcom/src/iocsh/iocsh.cpp
> @@ -586,18 +626,24 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
>      /*
>       * Check for existing macro context or construct a new one.

Adjust comment "macro context"

>       */
> -    handle = (MAC_HANDLE *) epicsThreadPrivateGet(iocshMacroHandleId);
> -    
> -    if (handle == NULL) {
> -        if (macCreateHandle(&handle, pairs)) {
> +    context = (iocshContext *) epicsThreadPrivateGet(iocshContextId);
> +
> +    if (!context) {
> +        context = (iocshContext*)calloc(1, sizeof(*context));
> +        if (!context || macCreateHandle(&context->handle, pairs)) {
>              errlogMessage("iocsh: macCreateHandle failed.");
>              free(redirects);
> +            free(context);
>              return -1;
>          }
>          
> -        epicsThreadPrivateSet(iocshMacroHandleId, (void *) handle);
> +        epicsThreadPrivateSet(iocshContextId, (void *) context);
>      }
> -    
> +    MAC_HANDLE *handle = context->handle;
> +
> +    scope.outer = context->scope;
> +    context->scope = &scope;
> +
>      macPushScope(handle);
>      macInstallMacros(handle, defines);
>      
> @@ -871,12 +930,38 @@ iocshBody (const char *pathname, const char *commandLine, const char *macros)
>              }
>          }
>          stopRedirect(filename, lineno, redirects);
> +
> +        if(!scope.interactive && scope.errored) {
> +            if(scope.onerr==Continue) {
> +                /* do nothing */
> +
> +            } else if(scope.onerr==Break) {
> +                ret = -1;
> +                fprintf(epicsGetStderr(), "iocsh Error: Break\n" );
> +                break;
> +
> +            } else if(scope.onerr==Halt) {
> +                ret = -1;
> +                if(scope.timeout<=0.0 || isinf(scope.timeout)) {
> +                    fprintf(epicsGetStderr(), "iocsh Error: Halt\n" );
> +                    epicsThreadSuspendSelf();
> +                    break;
> +
> +                } else {
> +                    fprintf(epicsGetStderr(), "iocsh Error: Waiting %f sec ...\n", scope.timeout);
> +                    epicsThreadSleep(scope.timeout);

The "on error wait <time>" function isn't working when another "on error" is used in a nested script. There is no delay between the "no such file" error and it printing "Success" when I run the "<test2.iocsh" with following scripts:

::::::::::::::
test2.iocsh
::::::::::::::
on error wait 10
<test.iocsh
echo Success!
::::::::::::::
test.iocsh
::::::::::::::
on error break
dbLoadRecords "no-file"
echo Hello
::::::::::::::

It does work without the nested script (although I don't think you need to print the delay time to 6 decimal places).

> +                }
> +            }
> +        }
>      }
>      macPopScope(handle);
>      
> -    if (handle->level == 0) {
> +    if (!scope.outer) {
>          macDeleteHandle(handle);
> -        epicsThreadPrivateSet(iocshMacroHandleId, NULL);
> +        free(context);
> +        epicsThreadPrivateSet(iocshContextId, NULL);
> +    } else {
> +        context->scope = scope.outer;
>      }
>      if (fp && (fp != stdin))
>          fclose (fp);
> @@ -1054,6 +1139,44 @@ static void iocshRunCallFunc(const iocshArgBuf *args)
>      iocshRun(args[0].sval, args[1].sval);
>  }
>  
> +/* on */
> +static const iocshArg onArg0 = { "'error' 'continue' | 'break' | 'wait' [value] | 'halt'", iocshArgArgv };
> +static const iocshArg *onArgs[1] = {&onArg0};
> +static const iocshFuncDef onFuncDef = {"on", 1, onArgs};
> +static void onCallFunc(const iocshArgBuf *args)
> +{
> +    iocshContext *context = (iocshContext *) epicsThreadPrivateGet(iocshContextId);
> +
> +    if(!context || !context->scope) {
> +        // we are not called through iocshBody()...
> +
> +    } else if(args->aval.ac<3 || strcmp(args->aval.av[1], "error")!=0) {
> +        fprintf(epicsGetStderr(), "Usage: on error [continue | break | halt | wait <delay>]\n");
> +
> +    } else if(context->scope->interactive) {
> +        fprintf(epicsGetStderr(), "Interactive shell ignores  on error ...\n");
> +
> +    } else if(strcmp(args->aval.av[2], "continue")==0) {
> +        context->scope->onerr = Continue;
> +
> +    } else if(strcmp(args->aval.av[2], "break")==0) {
> +        context->scope->onerr = Break;
> +
> +    } else if(strcmp(args->aval.av[2], "halt")==0) {
> +        context->scope->onerr = Halt;
> +        context->scope->timeout = 0.0;
> +
> +    } else if(strcmp(args->aval.av[2], "wait")==0) {
> +        context->scope->onerr = Halt;
> +        if(args->aval.ac==3 || epicsParseDouble(args->aval.av[3], &context->scope->timeout, NULL)) {
> +            context->scope->timeout = 5.0;
> +        }
> +
> +    } else {
> +        fprintf(epicsGetStderr(), "Usage: on error [continue | break | halt | wait <delay>]\n");

Should these generate the same output?

epics> help on
on 'error' 'continue' | 'break' | 'wait' [value] | 'halt'
epics> on garbage
Usage: on error [continue | break | halt | wait <delay>]

I prefer <delay> to [value], <seconds> might be even better:

epics> help epicsThreadSleep
epicsThreadSleep seconds

> +    }
> +}
> +
>  /*
>   * Dummy internal commands -- register and install in command table
>   * so they show up in the help display

-- 
https://code.launchpad.net/~epics-core/epics-base/+git/Com/+merge/366876
Your team EPICS Core Developers is subscribed to branch epics-base:7.0.

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System