Hiroyuki Kaguchi <fj7025cf@xxxxxxxxxxxxxxxxx> 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;
> }
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list