[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>

diff --git a/src/core/settings.c b/src/core/settings.c
index 54d48d326fecafaff90c56168b09721633eaed74..42caae0c530ce1c53f2740996e812da67d7c1433 100644 (file)
--- 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 028f8163b1502bbbd158eab48e3124c88c86ac22..3156f8d22b74331e5a76c66e1e6bead90323fdba 100644 (file)
--- 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,