[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Hi, Jim

Thank you very much for your comments.

Regards
Hiroyuki Kaguchi

On 2008/03/19 19:55, Jim Meyering wrote:
Hiroyuki Kaguchi <fj7025cf aa jp fujitsu com> 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;
+    }




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]