util/nvmutil: tidy up open_dev_urandom

Signed-off-by: Leah Rowe <leah@libreboot.org>

util/nvmutil/nvmutil.c

@@ -42,9 +42,9 @@ static void cmd_setmac(void);
 static void parse_mac_string(void);
 static void set_mac_byte(size_t mac_byte_pos);
 static void set_mac_nib(size_t mac_str_pos,
-   size_t mac_byte_pos, size_t mac_nib_pos);
-static uint8_t hextonum(char ch_s);
-static uint8_t rhex(void);
+    size_t mac_byte_pos, size_t mac_nib_pos);
+static uint16_t hextonum(char ch_s);
+static uint16_t rhex(void);
 static void read_file_exact(int fd, void *buf, size_t len,
     off_t off, const char *path, const char *op);
 static int write_mac_part(size_t partnum);
@@ -108,7 +108,7 @@ static void set_err(int errval);
 #define MAX_RETRY_READ 30
 
 /*
- * Portably macro based on BSD nitems.
+ * Portable macro based on BSD nitems.
  * Used to count the number of commands (see below).
  */
 #define items(x) (sizeof((x)) / sizeof((x)[0]))
@@ -266,16 +266,18 @@ set_cmd(int argc, char *argv[])
 	size_t i;
 
 	/*
-	 * Example: ./nvmutil gbe.bin
-	 *
-	 * Here, we assume that the user
-	 * wants a randomised MAC address.
+	 * No extra args: ./nvmutil gbe.bin
+	 * Equivalent: ./nvmutil gbe.bin setmac xx:xx:xx:xx:xx:xx
 	 */
 	if (argc == 2) {
 		cmd = cmd_setmac;
 		return;
 	}
 
+	/*
+	 * Three or more args.
+	 * Example: ./nvmutil gbe.bin copy 0
+	 */
 	for (i = 0; i < items(command); i++) {
 		if (strcmp(argv[2], command[i].str) != 0)
 			continue;
@@ -293,28 +295,14 @@ check_cmd_args(int argc, char *argv[])
 {
 	if (cmd == NULL && argc > 2) {
 		/*
-		 * Here, no valid command was found, but a
-		 * 3rd argument is available, which tells
-		 * us that the 3rd argument is a MAC address
-		 * supplied by the user, which could also
-		 * contain one or more random characters.
-		 *
-		 * This is intentional, because a lot of
-		 * users might run something like:
-		 *
-		 * ./nvmutil gbe.bin xx:1f:16:??:??:??
-		 *
-		 * Instead of (more properly):
-		 *
-		 * ./nvmutil gbe.bin setmac xx:1f:16:??:??:??
-		 *
-		 * This quirk makes the tool easier to use.
+		 * Example: ./nvmutil gbe.bin xx:1f:16:xx:xx:xx
+		 * Equivalent ./nvmutil gbe.bin setmac xx:1f:16:xx:xx:xx
 		 */
 		mac_str = argv[2];
 		cmd = cmd_setmac;
 	} else if (cmd == cmd_setmac) {
 		/*
-		 * ./nvmutil gbe.bin setmac [MAC]
+		 * Example: ./nvmutil gbe.bin setmac xx:1f:16:xx:xx:xx
 		 */
 		mac_str = rmac; /* random MAC */
 		if (argc > 3)
@@ -343,10 +331,12 @@ conv_argv_part_num(const char *part_str)
 	if (part_str[0] == '\0' || part_str[1] != '\0')
 		err(EINVAL, "Partnum string '%s' wrong length", part_str);
 
-	ch = (unsigned char)part_str[0] - '0';
+	ch = (unsigned char)part_str[0];
 
-	check_part_num((size_t)ch);
-	return (size_t)ch;
+	if (ch < '0' || ch > '1')
+		err(EINVAL, "Bad part number (%c)", ch);
+
+	return (size_t)(ch - '0');
 }
 
 static void
@@ -367,21 +357,24 @@ open_dev_urandom(void)
 {
 	struct stat st_urandom_fd;
 
+	/*
+	 * Try /dev/urandom first
+	 */
 	rname = newrandom;
+	if ((urandom_fd = open(rname, O_RDONLY)) != -1)
+		return;
 
-	if ((urandom_fd = open(rname, O_RDONLY)) == -1) {
-		/*
-		 * Fall back to /dev/random on old platforms
-		 * where /dev/urandom does not exist.
-		 *
-		 * We must reset the error condition first,
-		 * to prevent stale error status later.
-		 */
-		errno = 0;
+	/*
+	 * Fall back to /dev/random on old platforms
+	 * where /dev/urandom does not exist.
+	 *
+	 * We must reset the error condition first,
+	 * to prevent stale error status later.
+	 */
+	errno = 0;
 
-		rname = oldrandom;
-		xopen(&urandom_fd, rname, O_RDONLY, &st_urandom_fd);
-	}
+	rname = oldrandom;
+	xopen(&urandom_fd, rname, O_RDONLY, &st_urandom_fd);
 }
 #endif
 
@@ -462,7 +455,7 @@ static void
 cmd_setmac(void)
 {
 	size_t partnum;
-	unsigned char mac_updated = 0;
+	uint8_t mac_updated = 0;
 
 	parse_mac_string();
 	printf("MAC address to be written: %s\n", mac_str);
@@ -513,15 +506,14 @@ set_mac_byte(size_t mac_byte_pos)
 
 static void
 set_mac_nib(size_t mac_str_pos,
-   size_t mac_byte_pos, size_t mac_nib_pos)
+    size_t mac_byte_pos, size_t mac_nib_pos)
 {
 	char mac_ch;
 	uint16_t hex_num;
 
 	mac_ch = mac_str[mac_str_pos + mac_nib_pos];
 
-	hex_num = hextonum(mac_ch);
-	if (hex_num > 15)
+	if ((hex_num = hextonum(mac_ch)) > 15)
 		err(EINVAL, "Invalid character '%c'",
 		    mac_str[mac_str_pos + mac_nib_pos]);
 
@@ -547,7 +539,7 @@ set_mac_nib(size_t mac_str_pos,
 	    | ((mac_nib_pos ^ 1) << 2)); /* left or right nib? */
 }
 
-static uint8_t
+static uint16_t
 hextonum(char ch_s)
 {
 	/*
@@ -563,13 +555,14 @@ hextonum(char ch_s)
 
 	if ((unsigned)(ch - 'a') <= 5)
 		return ch - 'a' + 10;
-	else if (ch == '?' || ch == 'x')
+
+	if (ch == '?' || ch == 'x')
 		return rhex(); /* random character */
-	else
-		return 16; /* invalid character */
+
+	return 16; /* invalid character */
 }
 
-static uint8_t
+static uint16_t
 rhex(void)
 {
 	static size_t n = 0;
@@ -584,7 +577,7 @@ rhex(void)
 #endif
 	}
 
-	return rnum[--n] & 0xf;
+	return (uint16_t)(rnum[--n] & 0xf);
 }
 
 static void