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