From caf064e17bbdfa0d3a0149b61c4f427ebb4fee8c Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Fri, 12 Aug 2016 10:37:41 -0600 Subject: [PATCH] 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. --- plugins/sudoers/defaults.c | 181 +++++++++---------------------------- 1 file changed, 45 insertions(+), 136 deletions(-) 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 * -- 2.40.0