Re: [Libvir] [PATCH] Fix MAC address parsing for 1-digit case
Hiroyuki Kaguchi <fj7025cf(a)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;
}