Refactor the error parts of set_default_entry() so the switch() is

mostly just calls to store_foo() functions.  Avoids a lot of
duplicated error checking and silences a cppcheck false positive.

diff --git a/plugins/sudoers/defaults.c b/plugins/sudoers/defaults.c
index 3ac489e7656c6446602f5f3733b47ff6e05fdfde..2643d684b06278851c055c97048b96164922baf2 100644 (file)
--- a/plugins/sudoers/defaults.c
+++ b/plugins/sudoers/defaults.c
@@ -190,185 +190,94 @@ dump_defaults(void)
 }
 
 static bool
-set_default_entry(struct sudo_defs_types *def, const char *val, int op, bool quiet, bool do_callback)
+set_default_entry(struct sudo_defs_types *def, const char *val, int op,
+    bool quiet, bool do_callback)
 {
+    int rc;
     debug_decl(set_default_entry, SUDOERS_DEBUG_DEFAULTS)
 
+    if (val == NULL) {
+       /* Check for bogus boolean usage or missing value if non-boolean. */
+       if (!ISSET(def->type, T_BOOL) || op != false) {
+           if (!quiet)
+               sudo_warnx(U_("no value specified for `%s'"), def->name);
+           debug_return_bool(false);
+       }
+    }
+
     switch (def->type & T_MASK) {
        case T_LOGFAC:
-           if (!store_syslogfac(val, def, op)) {
-               if (!quiet) {
-                   if (val)
-                       sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                           val, def->name);
-                   else
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_syslogfac(val, def, op);
            break;
        case T_LOGPRI:
-           if (!store_syslogpri(val, def, op)) {
-               if (!quiet) {
-                   if (val)
-                       sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                           val, def->name);
-                   else
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_syslogpri(val, def, op);
            break;
        case T_STR:
-           if (!val) {
-               /* Check for bogus boolean usage or lack of a value. */
-               if (!ISSET(def->type, T_BOOL) || op != false) {
-                   if (!quiet)
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-                   debug_return_bool(false);
-               }
-           }
-           if (ISSET(def->type, T_PATH) && val && *val != '/') {
+           if (ISSET(def->type, T_PATH) && val != NULL && *val != '/') {
                if (!quiet) {
                    sudo_warnx(U_("values for `%s' must start with a '/'"),
                        def->name);
                }
-               debug_return_bool(false);
-           }
-           switch (store_str(val, def, op)) {
-           case true:
-               /* OK */
+               rc = -1;
                break;
-           case false:
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               /* FALLTHROUGH */
-           default:
-               debug_return_bool(false);
            }
+           rc =  store_str(val, def, op);
            break;
        case T_INT:
-           if (!val) {
-               /* Check for bogus boolean usage or lack of a value. */
-               if (!ISSET(def->type, T_BOOL) || op != false) {
-                   if (!quiet)
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-                   debug_return_bool(false);
-               }
-           }
-           if (!store_int(val, def, op)) {
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_int(val, def, op);
            break;
        case T_UINT:
-           if (!val) {
-               /* Check for bogus boolean usage or lack of a value. */
-               if (!ISSET(def->type, T_BOOL) || op != false) {
-                   if (!quiet)
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-                   debug_return_bool(false);
-               }
-           }
-           if (!store_uint(val, def, op)) {
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_uint(val, def, op);
            break;
        case T_FLOAT:
-           if (!val) {
-               /* Check for bogus boolean usage or lack of a value. */
-               if (!ISSET(def->type, T_BOOL) || op != false) {
-                   if (!quiet)
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-                   debug_return_bool(false);
-               }
-           }
-           if (!store_float(val, def, op)) {
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_float(val, def, op);
            break;
        case T_MODE:
-           if (!val) {
-               /* Check for bogus boolean usage or lack of a value. */
-               if (!ISSET(def->type, T_BOOL) || op != false) {
-                   if (!quiet)
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-                   debug_return_bool(false);
-               }
-           }
-           if (!store_mode(val, def, op)) {
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_mode(val, def, op);
            break;
        case T_FLAG:
-           if (val) {
+           if (val != NULL) {
                if (!quiet) {
                    sudo_warnx(U_("option `%s' does not take a value"),
                        def->name);
                }
-               debug_return_bool(false);
+               rc = -1;
+               break;
            }
            def->sd_un.flag = op;
            break;
        case T_LIST:
-           if (!val) {
-               /* Check for bogus boolean usage or lack of a value. */
-               if (!ISSET(def->type, T_BOOL) || op != false) {
-                   if (!quiet)
-                       sudo_warnx(U_("no value specified for `%s'"), def->name);
-                   debug_return_bool(false);
-               }
-           }
-           if (!store_list(val, def, op)) {
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_list(val, def, op);
            break;
        case T_TUPLE:
-           if (!val && !ISSET(def->type, T_BOOL)) {
-               if (!quiet)
-                   sudo_warnx(U_("no value specified for `%s'"), def->name);
-               debug_return_bool(false);
-           }
-           if (!store_tuple(val, def, op)) {
-               if (!quiet) {
-                   sudo_warnx(U_("value `%s' is invalid for option `%s'"),
-                       val, def->name);
-               }
-               debug_return_bool(false);
-           }
+           rc = store_tuple(val, def, op);
            break;
        default:
            if (!quiet) {
                sudo_warnx(U_("invalid Defaults type 0x%x for option `%s'"),
                    def->type, def->name);
            }
-           debug_return_bool(false);
+           rc = -1;
+           break;
+    }
+    switch (rc) {
+    case -1:
+       /* Error message already displayed. */
+       rc = false;
+       break;
+    case false:
+       if (!quiet) {
+           sudo_warnx(U_("value `%s' is invalid for option `%s'"),
+               val, def->name);
+       }
+       break;
+    case true:
+       if (do_callback && def->callback)
+           rc = def->callback(&def->sd_un);
+       break;
     }
-    if (do_callback && def->callback)
-       debug_return_bool(def->callback(&def->sd_un));
 
-    debug_return_bool(true);
+    debug_return_bool(rc);
 }
 
 struct early_default *