Hi, Jim
Thank you very much for your comments.
Regards
Hiroyuki Kaguchi
On 2008/03/19 19:55, Jim Meyering wrote:
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