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

Hiroyuki Kaguchi <fj7025cf(a)aa.jp.fujitsu.com&gt; wrote: > Thank you for your suggestions. > I apply the following changes to the patch. > 1) Initialize errno to 0 before the loop. > 2) Use isxdigit instead of isdigit and isalpha, and add a comment. > 3) Test errno != 0. > 4) Add a terminating NUL check. > 5) the MAC address function go in file src/util.c. Looks good. A couple nits. I didn't read the comments the first time. > +/** > + * virParseMacAddr: > + * @str: pointer to the char used > + * @addr: MAC address * @str: string representation of MAC address, e.g., "0:1E:FC:E:3a:CB" * @addr: 6-byte MAC address > + * Parse a MAC address > + * > + * Returns 0 or -1 in case of error. * Return 0 upon success, or -1 in case of error. > + */ > +int > +virParseMacAddr(const char* str, unsigned char *addr) > +{ > + int i; > + > + errno = 0; > + for (i = 0; i < 6; i++) { > + char *end_ptr; > + unsigned long result; > + > + /* 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 != 0) || > + (0xFF < result)) > + break; > + > + addr[i] = (unsigned char) result; > + > + if (*end_ptr != ':') { > + if (i == 5 && *end_ptr == '\0') > + return 0; > + > + break; > + } No big deal, but I find this to be more readable: if (i == 5 && *end_ptr == '\0') return 0; if (*end_ptr != ':') break; and with any compiler optimization at all, it's just as efficient. > + str = end_ptr + 1; > + }