From 7774ceed2f57d3711b29ccc464e6c88f20e3c83b Mon Sep 17 00:00:00 2001
From: Michael Brown <mcb30@ipxe.org>
Date: Fri, 12 Jul 2013 14:45:55 +0200
Subject: [PATCH] [settings] Use hex_decode() to parse hex settings

Use hex_decode() to parse "hex" and "hexhyp" settings.  Note that this
parser is stricter than the old parser; it now requires exactly two
hex digits for each byte.  (The old parser was based upon strtoul()
and so would allow leading whitespace and a leading plus or minus
sign.)

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/core/settings.c       | 61 +++++++++++++++++++--------------------
 src/tests/settings_test.c | 16 +++++-----
 2 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/src/core/settings.c b/src/core/settings.c
index 54d48d32..42caae0c 100644
--- a/src/core/settings.c
+++ b/src/core/settings.c
@@ -32,6 +32,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/dhcp.h>
 #include <ipxe/uuid.h>
 #include <ipxe/uri.h>
+#include <ipxe/base16.h>
 #include <ipxe/init.h>
 #include <ipxe/settings.h>
 
@@ -1685,37 +1686,6 @@ struct setting_type setting_type_uint32 __setting_type = {
 	.format = format_uint_setting,
 };
 
-/**
- * Parse hex string setting value
- *
- * @v value		Formatted setting value
- * @v buf		Buffer to contain raw value
- * @v len		Length of buffer
- * @ret len		Length of raw value, or negative error
- */
-static int parse_hex_setting ( const char *value, void *buf, size_t len ) {
-	char *ptr = ( char * ) value;
-	uint8_t *bytes = buf;
-	unsigned int count = 0;
-	uint8_t byte;
-
-	while ( 1 ) {
-		byte = strtoul ( ptr, &ptr, 16 );
-		if ( count++ < len )
-			*bytes++ = byte;
-		switch ( *ptr ) {
-		case '\0' :
-			return count;
-		case ':' :
-		case '-' :
-			ptr++;
-			break;
-		default :
-			return -EINVAL;
-		}
-	}
-}
-
 /**
  * Format hex string setting value
  *
@@ -1742,6 +1712,19 @@ static int format_hex_setting ( const void *raw, size_t raw_len, char *buf,
 	return used;
 }
 
+/**
+ * Parse hex string setting value (using colon delimiter)
+ *
+ * @v value		Formatted setting value
+ * @v buf		Buffer to contain raw value
+ * @v len		Length of buffer
+ * @v size		Integer size, in bytes
+ * @ret len		Length of raw value, or negative error
+ */
+static int parse_hex_setting ( const char *value, void *buf, size_t len ) {
+	return hex_decode ( value, ':', buf, len );
+}
+
 /**
  * Format hex string setting value (using colon delimiter)
  *
@@ -1756,6 +1739,20 @@ static int format_hex_colon_setting ( const void *raw, size_t raw_len,
 	return format_hex_setting ( raw, raw_len, buf, len, ":" );
 }
 
+/**
+ * Parse hex string setting value (using hyphen delimiter)
+ *
+ * @v value		Formatted setting value
+ * @v buf		Buffer to contain raw value
+ * @v len		Length of buffer
+ * @v size		Integer size, in bytes
+ * @ret len		Length of raw value, or negative error
+ */
+static int parse_hex_hyphen_setting ( const char *value, void *buf,
+				      size_t len ) {
+	return hex_decode ( value, '-', buf, len );
+}
+
 /**
  * Format hex string setting value (using hyphen delimiter)
  *
@@ -1780,7 +1777,7 @@ struct setting_type setting_type_hex __setting_type = {
 /** A hex-string setting (hyphen-delimited) */
 struct setting_type setting_type_hexhyp __setting_type = {
 	.name = "hexhyp",
-	.parse = parse_hex_setting,
+	.parse = parse_hex_hyphen_setting,
 	.format = format_hex_hyphen_setting,
 };
 
diff --git a/src/tests/settings_test.c b/src/tests/settings_test.c
index 028f8163..3156f8d2 100644
--- a/src/tests/settings_test.c
+++ b/src/tests/settings_test.c
@@ -51,9 +51,15 @@ FILE_LICENCE ( GPL2_OR_LATER );
 	ok ( storef_setting ( settings, setting, formatted ) == 0 );	\
 	len = fetch_setting ( settings, setting, actual,		\
 			      sizeof ( actual ) );			\
-	DBGC ( settings, "Stored %s \"%s\", got:\n",			\
-	       (setting)->type->name, formatted );			\
-	DBGC_HDA ( settings, 0, actual, len );				\
+	if ( len >= 0 ) {						\
+		DBGC ( settings, "Stored %s \"%s\", got:\n",		\
+		       (setting)->type->name, formatted );		\
+		DBGC_HDA ( settings, 0, actual, len );			\
+	} else {							\
+		DBGC ( settings, "Stored %s \"%s\", got error %s\n",	\
+		       (setting)->type->name, formatted,		\
+		       strerror ( len ) );				\
+	}								\
 	ok ( len == ( int ) sizeof ( actual ) );			\
 	ok ( memcmp ( actual, expected, sizeof ( actual ) ) == 0 );	\
 	} while ( 0 )
@@ -239,10 +245,6 @@ static void settings_test_exec ( void ) {
 		    RAW ( 0xf2, 0x37, 0xb2, 0x18 ), "0xf237b218" );
 
 	/* "hex" setting type */
-	storef_ok ( &test_settings, &test_hex_setting,
-		    ":", RAW ( 0x00, 0x00 ) );
-	storef_ok ( &test_settings, &test_hex_setting,
-		    "1:2:", RAW ( 0x01, 0x02, 0x00 ) );
 	storef_ok ( &test_settings, &test_hex_setting,
 		    "08:12:f5:22:90:1b:4b:47:a8:30:cb:4d:67:4c:d6:76",
 		    RAW ( 0x08, 0x12, 0xf5, 0x22, 0x90, 0x1b, 0x4b, 0x47, 0xa8,