From e3fc775d26c9650bd6fbde30353d66fec12811b9 Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <alexander@amelkin.msk.ru>
Date: Wed, 10 Jun 2020 00:54:15 +0300
Subject: [PATCH] channel: Refactor set_user_access option processing

Reduce code duplication by extracting option names, types,
and value ranges into a separate structure, and rewriting
the option parsing code without mixing it with the data.

Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
 include/ipmitool/ipmi_constants.h |  1 +
 lib/ipmi_channel.c                | 86 +++++++++++++++++++++----------
 lib/ipmi_strings.c                |  4 +-
 3 files changed, 62 insertions(+), 29 deletions(-)

diff --git a/include/ipmitool/ipmi_constants.h b/include/ipmitool/ipmi_constants.h
index 5391d4f..fc8642d 100644
--- a/include/ipmitool/ipmi_constants.h
+++ b/include/ipmitool/ipmi_constants.h
@@ -74,6 +74,7 @@
 #define IPMI_SESSION_PRIV_OPERATOR	0x3
 #define IPMI_SESSION_PRIV_ADMIN		0x4
 #define IPMI_SESSION_PRIV_OEM		0x5
+#define IPMI_SESSION_PRIV_NOACCESS	0xF
 
 #define IPMI_SET_IN_PROGRESS_SET_COMPLETE 0x00
 #define IPMI_SET_IN_PROGRESS_IN_PROGRESS  0x01
diff --git a/lib/ipmi_channel.c b/lib/ipmi_channel.c
index 2886899..ac3cf2d 100644
--- a/lib/ipmi_channel.c
+++ b/lib/ipmi_channel.c
@@ -803,8 +803,26 @@ ipmi_set_user_access(struct ipmi_intf *intf, int argc, char **argv)
 	int ccode = 0;
 	int i = 0;
 	uint8_t channel = 0;
-	uint8_t priv = 0;
 	uint8_t user_id = 0;
+	struct {
+		const char *option;
+		enum {
+			UA_INTEGER, /* direct integer value */
+			UA_BOOLEAN,  /* off/disable = false, on/enable = true */
+			UA_BOOLEAN_INVERSE /* off/disable = true, on/enable = false */
+		} type;
+		uint8_t *val;
+		uint8_t min; /* minimum value for UA_INTEGER options */
+		uint8_t max; /* maximum value for UA_INTEGER options */
+	} options[] = {
+		{ "callin=", UA_BOOLEAN_INVERSE, &user_access.callin_callback, 0, 0},
+		{ "link=", UA_BOOLEAN,  &user_access.link_auth, 0, 0},
+		{ "ipmi=", UA_BOOLEAN, &user_access.ipmi_messaging, 0, 0},
+		{ "privilege=", UA_INTEGER, &user_access.privilege_limit
+		              , IPMI_SESSION_PRIV_CALLBACK
+		              , IPMI_SESSION_PRIV_NOACCESS },
+	};
+
 	if (argc > 0 && strcmp(argv[0], "help") == 0) {
 		printf_channel_usage();
 		return 0;
@@ -827,33 +845,47 @@ ipmi_set_user_access(struct ipmi_intf *intf, int argc, char **argv)
 		return (-1);
 	}
 	for (i = 2; i < argc; i ++) {
-		if (strcmp(argv[i], "callin=") == 0) {
-			if (strcmp(argv[i] + strlen("callin="), "off") == 0) {
-				user_access.callin_callback = 1;
-			} else {
-				user_access.callin_callback = 0;
+		size_t j;
+		for (j = 0; j < ARRAY_SIZE(options); ++j) {
+			const char *opt = argv[i];
+			const int optlen = strlen(options[j].option);
+			if (!strncmp(opt, options[j].option, optlen)) {
+				const char *optval = opt + optlen;
+				uint16_t val;
+
+				if (UA_INTEGER != options[j].type) {
+					bool boolval = (UA_BOOLEAN_INVERSE == options[j].type)
+						? false
+						: true;
+					*options[j].val = boolval;
+					if (!strcmp(optval, "off")
+						|| !strcmp(optval, "disable")
+						|| !strcmp(optval, "no")
+					   )
+					{
+						boolval = !boolval;
+					}
+				} else if (UINT8_MAX
+				           != (val = str2val(optval, ipmi_privlvl_vals)))
+				{
+					*options[j].val = (uint8_t)val;
+				} else if (str2uchar(optval, options[j].val)) {
+						lprintf(LOG_ERR
+						        , "Numeric [%hhu-%hhu] value expected, "
+						          "but '%s' given."
+						        , options[j].min
+						        , options[j].max
+						        , optval);
+						return (-1);
+				}
+				lprintf(LOG_DEBUG
+				        , "Option %s=%hhu"
+				        , options[j].option
+				        , *options[j].val);
+				break;
 			}
-		} else if (strcmp(argv[i], "link=") == 0) {
-			if (strcmp(argv[i] + strlen("link="), "off") == 0) {
-				user_access.link_auth = 0;
-			} else {
-				user_access.link_auth = 1;
-			}
-		} else if (strcmp(argv[i], "ipmi=") == 0) {
-			if (strcmp(argv[i] + strlen("ipmi="), "off") == 0) {
-				user_access.ipmi_messaging = 0;
-			} else {
-				user_access.ipmi_messaging = 1;
-			}
-		} else if (strcmp(argv[i], "privilege=") == 0) {
-			if (str2uchar(argv[i] + 10, &priv) != 0) {
-				lprintf(LOG_ERR,
-						"Numeric value expected, but '%s' given.",
-						argv[i] + 10);
-				return (-1);
-			}
-			user_access.privilege_limit = priv;
-		} else {
+		}
+		if (ARRAY_SIZE(options) == j) {
 			lprintf(LOG_ERR, "Invalid option: %s\n", argv[i]);
 			return (-1);
 		}
diff --git a/lib/ipmi_strings.c b/lib/ipmi_strings.c
index aab50f6..9fc6b61 100644
--- a/lib/ipmi_strings.c
+++ b/lib/ipmi_strings.c
@@ -1031,8 +1031,8 @@ const struct valstr ipmi_privlvl_vals[] = {
 	{ IPMI_SESSION_PRIV_OPERATOR,	"OPERATOR" },
 	{ IPMI_SESSION_PRIV_ADMIN,	    "ADMINISTRATOR" },
 	{ IPMI_SESSION_PRIV_OEM,    	"OEM" },
-	{ 0xF,	        		    	"NO ACCESS" },
-	{ 0xFF,			             	NULL },
+	{ IPMI_SESSION_PRIV_NOACCESS, "NO ACCESS" },
+	{ UINT8_MAX, NULL },
 };