Re: [Libvir] [PATCH] Fix MAC address parsing for 1-digit case

18 Mar 2008

      Hiroyuki Kaguchi <fj7025cf@aa.jp.fujitsu.com> wrote:
...
libvirt fails in parsing when MAC address like "00:16:3e:12:3:61"
is specified for installation.
This is because virt-install can pass 1-digit (like 3) for
MAC address from Cset:316 for Solaris
But libvirt cannot support this MAC 1-digit (like 3) parameter.
Thanks for the patch.
Any change that eliminates a sscanf-based parser is a good one ;-)

A few suggestions:

...
...
+static int
+parseMacAddr(const char* str, unsigned char *addr)
+{
+    int i;
Initialize errno to 0 before the loop.
Otherwise, an incoming nonzero value could cause rejection
of a valid MAC address.
...
+    for (i = 0; i < 6; i++) {
+        char *end_ptr;
+        unsigned long result;
+
+        if (!isdigit(*str) && !isalpha(*str))
+            break;
Use isxdigit instead, and add a comment
saying this is solely to avoid accepting the leading
space or "+" that strtoul would otherwise accept.

           if (!isxdigit(*str))
               break;
...
+
+        result = strtoul(str, &end_ptr, 16);
+
+        if ((end_ptr - str) < 1 || 2 < (end_ptr - str) ||
+            (errno == ERANGE) ||
While it's ok to test errno == ERANGE,
testing errno != 0 is a little better.
...
+            (0xFF < result))
+            break;
+
+        addr[i] = (unsigned char) result;
The following test ensures that there is a ":" between adjacent
pairs, but does not require a terminating NUL byte.  That means
it would accept arbitrary text after a valid sixth component,
e.g., "00:16:3e:12:3:61-anything-at-all" or a less-contrived
s/0/O/ typo, e.g., "00:16:3e:12:3:6O"
...
+        if (*end_ptr != ':')
+            return (i == 5) ? 0 : -1;
You can use this instead:

           if (*end_ptr != (i < 5 ? ':' : '\0'))
               return -1;
           if (i == 5)
               return 0;

...
...
virBufferAddLit(buf, "(vif ");
     if (mac != NULL) {
-        unsigned int addr[12];
-        int tmp = sscanf((const char *) mac,
-		     "%01x%01x:%01x%01x:%01x%01x:%01x%01x:%01x%01x:%01x%01x",
-                         (unsigned int *) &addr[0],
-                         (unsigned int *) &addr[1],
-                         (unsigned int *) &addr[2],
-                         (unsigned int *) &addr[3],
-                         (unsigned int *) &addr[4],
-                         (unsigned int *) &addr[5],
-                         (unsigned int *) &addr[6],
-                         (unsigned int *) &addr[7],
-                         (unsigned int *) &addr[8],
-                         (unsigned int *) &addr[9],
-                         (unsigned int *) &addr[10],
-                         (unsigned int *) &addr[11]);
-        if (tmp != 12 || strlen((const char *) mac) != 17) {
+        unsigned char addr[6];
+        if (parseMacAddr((const char*) mac, addr) == -1) {
             virXMLError(conn, VIR_ERR_INVALID_MAC, (const char *) mac, 0);
             goto error;
         }

Re: [Libvir] [PATCH] Fix MAC address parsing for 1-digit case

Jim Meyering