From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Fri, 12 Aug 2016 16:37:41 +0000 (-0600)
Subject: Refactor the error parts of set_default_entry() so the switch() is
X-Git-Tag: SUDO_1_8_18^2~77
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=caf064e17bbdfa0d3a0149b61c4f427ebb4fee8c;p=sudo

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 3ac489e76..2643d684b 100644
--- 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 *