Error handling for CLI

Continue, do not exit on read/expand errors
Accept -1 error without clicon_err in callbacks / expand
C-API: Three-value return of clicon_cliread

apps/cli/cli_main.c

@@ -156,7 +156,6 @@ cli_history_save(clicon_handle h)
     return retval;
 }
 
-
 /*! Clean and close all state of cli process (but dont exit). 
  * Cannot use h after this 
  * @param[in]  h  Clixon handle
@@ -183,7 +182,7 @@ cli_terminate(clicon_handle h)
     /* Delete all plugins, and RPC callbacks */
     clixon_plugin_module_exit(h);
     /* Delete CLI syntax et al */
-    cli_plugin_finish(h);  
+    cli_plugin_finish(h);
 
     cli_history_save(h);
     cli_handle_exit(h);
@@ -208,7 +207,7 @@ static int
 cli_signal_init (clicon_handle h)
 {
     int retval = -1;
-    
+
     cli_signal_block(h);
     if (set_signal(SIGTERM, cli_sig_term, NULL) < 0){
         clicon_err(OE_UNIX, errno, "Setting SIGTERM signal");
@@ -236,17 +235,22 @@ cli_interactive(clicon_handle h)
     char         *cmd;
     char         *new_mode;
     cligen_result result;
-    
+    int           ret;
+
     /* Loop through all commands */
     while(!cligen_exiting(cli_cligen(h))) {
         new_mode = cli_syntax_mode(h);
         cmd = NULL;
-        if (clicon_cliread(h, &cmd) < 0)
+        /* Read a CLI string, including expand handling */
+        if ((ret = clicon_cliread(h, &cmd)) < 0)
             goto done;
+        if (ret == 0)
+            continue;
         if (cmd == NULL) { /* EOF */
-            cligen_exiting_set(cli_cligen(h), 1); 
+            cligen_exiting_set(cli_cligen(h), 1);
             continue;
         }
+        /* Here errors are handled */
         if (clicon_parse(h, cmd, &new_mode, &result, NULL) < 0)
             goto done;
         /* Why not check result? */
@@ -265,7 +269,7 @@ autocli_trees_default(clicon_handle h)
     int         retval = -1;
     cbuf       *cb = NULL;
     int         mode = 0;
-    parse_tree *pt = NULL; 
+    parse_tree *pt = NULL;
     pt_head    *ph;
 
     /* Create backward compatible tree: @datamodel */
@@ -446,7 +450,7 @@ main(int    argc,
      char **argv)
 {
     int            retval = -1;
-    int            c;    
+    int            c;
     int            once;
     char          *tmp;
     char          *argv0 = argv[0];
@@ -470,7 +474,7 @@ main(int    argc,
     once = 0;
 
     /* In the startup, logs to stderr & debug flag set later */
-    clicon_log_init(__PROGRAM__, LOG_INFO, logdst); 
+    clicon_log_init(__PROGRAM__, LOG_INFO, logdst);
 
     /* Initiate CLICON handle. CLIgen is also initialized */
     if ((h = cli_handle_init()) == NULL)
@@ -502,7 +506,7 @@ main(int    argc,
                But this means that we need to check if 'help' is set before 
                exiting, and then call usage() before exit.
             */
-            help = 1; 
+            help = 1;
             break;
         case 'D' : /* debug */
             if (sscanf(optarg, "%d", &dbg) != 1)
@@ -531,7 +535,7 @@ main(int    argc,
      * Logs, error and debug to stderr or syslog, set debug level
      */
     clicon_log_init(__PROGRAM__, dbg?LOG_DEBUG:LOG_INFO, logdst);
-    clicon_debug_init(dbg, NULL); 
+    clicon_debug_init(dbg, NULL);
     yang_init(h);
 
     /* Find, read and parse configfile */
@@ -555,7 +559,7 @@ main(int    argc,
                 fprintf(stderr, "freopen: %s\n", strerror(errno));
                 return -1;
             }
-            break; 
+            break;
         case '1' : /* Quit after reading database once - dont wait for events */
             once = 1;
             break;
@@ -672,7 +676,7 @@ main(int    argc,
        Should be 0 but default is 1 since all legacy apps use 1
        Test legacy before shifting default to 0
      */
-    cligen_exclude_keys_set(cli_cligen(h), clicon_cli_varonly(h)); 
+    cligen_exclude_keys_set(cli_cligen(h), clicon_cli_varonly(h));
 
     /* Initialize plugin module by creating a handle holding plugin and callback lists */
     if (clixon_plugin_module_init(h) < 0)
@@ -702,7 +706,7 @@ main(int    argc,
     /* Create top-level and store as option */
     if ((yspec = yspec_new()) == NULL)
         goto done;
-    clicon_dbspec_yang_set(h, yspec);   
+    clicon_dbspec_yang_set(h, yspec);
     
     /* Load Yang modules
      * 1. Load a yang module as a specific absolute filename */
@@ -815,7 +819,6 @@ main(int    argc,
             goto done;
     }
 
-
     /* Go into event-loop unless -1 command-line */
     if (!once){
         retval = cli_interactive(h);

apps/cli/cli_plugin.c

@@ -516,15 +516,17 @@ cli_plugin_finish(clicon_handle h)
 int 
 cli_handler_err(FILE *f)
 {
-    if (clicon_errno &&
+    if (clicon_errno){
-        (clicon_get_logflags() & CLICON_LOG_STDERR) == 0){
+        /* Check if error is already logged on stderr */
-        fprintf(f,  "%s: %s", clicon_strerror(clicon_errno), clicon_err_reason);
+        if ((clicon_get_logflags() & CLICON_LOG_STDERR) == 0){
-        if (clicon_suberrno)
+            fprintf(f,  "%s: %s", clicon_strerror(clicon_errno), clicon_err_reason);
-            fprintf(f, ": %s", strerror(clicon_suberrno));
+            if (clicon_suberrno)
-        fprintf(f,  "\n");
+                fprintf(f, ": %s", strerror(clicon_suberrno));
+            fprintf(f,  "\n");
+        }
+        else
+            fprintf(f, "CLI command error\n");
     }
-    else
-        fprintf(f, "CLI command error\n");
     return 0;
 }
 
@@ -595,8 +597,8 @@ clicon_parse(clicon_handle  h,
         clicon_debug(1, "%s result:%d command: \"%s\"", __FUNCTION__, *result, cmd);
         switch (*result) {
         case CG_EOF: /* eof */
-        case CG_ERROR:
+        case CG_ERROR: 
-            fprintf(f, "CLI parse error: %s\n", cmd);
+            fprintf(f, "CLI parse error: %s\n", cmd); // In practice never happens
             break;
         case CG_NOMATCH: /* no match */
             fprintf(f, "CLI syntax error: \"%s\": %s\n", cmd, reason);
@@ -741,7 +743,8 @@ cli_prompt_get(clicon_handle h,
 /*! Read command from CLIgen's cliread() using current syntax mode.
  * @param[in]  h       Clicon handle
  * @param[out] stringp Pointer to command buffer or NULL on EOF
- * @retval     0       OK
+ * @retval     1       OK
+ * @retval     0       Fail but continue
  * @retval    -1       Error
  */
 int
@@ -755,7 +758,7 @@ clicon_cliread(clicon_handle h,
     cli_prompthook_t *fn;
     clixon_plugin_t  *cp;
     char             *promptstr;
-    
+
     stx = cli_syntax(h);
     mode = stx->stx_active_mode;
     /* Get prompt from plugin callback? */
@@ -776,15 +779,22 @@ clicon_cliread(clicon_handle h,
     }
     cligen_ph_active_set_byname(cli_cligen(h), mode->csm_name);
 
+    clicon_err_reset();
     if (cliread(cli_cligen(h), stringp) < 0){
-        clicon_err(OE_FATAL, errno, "CLIgen");
+        cli_handler_err(stdout);
-        goto done;
+        if (clicon_suberrno == ESHUTDOWN)
+            goto done;
+        goto fail;
     }
-    retval = 0;
+
+    retval = 1;
  done:
     if (pfmt)
         free(pfmt);
     return retval;
+ fail:
+    retval = 0;
+    goto done;
 }
 
 /*

lib/clixon/clixon_err.h

@@ -101,7 +101,7 @@ extern char clicon_err_reason[ERR_STRLEN];
 /*
  * Macros
  */
-#define clicon_err(e,s,_fmt, args...)  clicon_err_fn(__FUNCTION__, __LINE__, (e), (s), _fmt , ##args)
+#define clicon_err(e,s,_fmt, args...) clicon_err_fn(__FUNCTION__, __LINE__, (e), (s), _fmt , ##args)
 
 /*
  * Prototypes

lib/src/clixon_err.c

@@ -97,8 +97,8 @@ static clixon_err_cats *_err_cat_list = NULL;
 /*
  * Variables
  */
-int  clicon_errno  = 0;    /* See enum clicon_err XXX: hide this and change to err_category */
+int  clicon_errno         = 0; /* See enum clicon_err XXX: hide this and change to err_category */
-int  clicon_suberrno  = 0; /* Corresponds to errno.h XXX: change to errno */
+int  clicon_suberrno      = 0; /* Corresponds to errno.h XXX: change to errno */
 char clicon_err_reason[ERR_STRLEN] = {0, };
 
 /*
@@ -155,6 +155,10 @@ clicon_err_reset(void)
 }
 
 /*! Find error category struct given name
+ *
+ * @param[in] category Error category
+ * @retval    cec      error category callback structs
+ * @retval    NULL     Not found
  */
 static struct clixon_err_cats *
 find_category(int category)
@@ -181,6 +185,13 @@ find_category(int category)
  * - Logs to syslog with LOG_ERR
  * - Set global error variable name clicon_errno
  * - Set global reason string clicon_err_reason
+ * Typically a call to clicon_err() is followed by a return -1 (or NULL) that signals 
+ * a fatal error that fails early and loud.
+ * However there are some cases where such an error does not cause an exit. This includes
+ * CLI operations of callbacks and expand functions. The reason is that user-defined errors
+ * should just signal an error and not terminate. To override this one can set a suberr to 
+ * ESHUTDOWN.
+ *
  * @note: err direction (syslog and/or stderr) controlled by clicon_log_init()
  *
  * @param[in]    fn       Inline function name (when called from clicon_err() macro)
@@ -227,7 +238,6 @@ clicon_err_fn(const char *fn,
     }
     va_end(args);
     strncpy(clicon_err_reason, msg, ERR_STRLEN-1);
-
     /* Check category callbacks as defined in clixon_err_cat_reg */
     if ((cec = find_category(category)) != NULL &&
         cec->cec_logfn){
@@ -282,7 +292,7 @@ clicon_err_fn(const char *fn,
                        msg);
     }
     retval = 0;
-  done:
+ done:
     if (msg)
         free(msg);
     return retval;