On 10/08/2014 09:36 AM, Laine Stump wrote:

On 10/06/2014 05:37 PM, akrowiak@linux.vnet.ibm.com wrote:

From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>

This patch provides the utility functions to needed to synchronize the
changes made to a guest domain network device's multicast filter
with the corresponding macvtap device's filter on the host:

* Get/add/remove multicast MAC addresses
* Get the macvtap device's RX filter list

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---

+/**
+ * virMacAddrParseHex:
+ * @str: string hexadecimal representation of MAC address, e.g., "F801EFCE3aCB"
+ * @addr: 6-byte MAC address
+ *
+ * Parse the hexadecimal representation of a MAC address
+ *
+ * Return 0 upon success, or -1 in case of error.
+ */
+int
+virMacAddrParseHex(const char* str, virMacAddrPtr addr)

Wrong spacing around '*'.

+{
+    if (strlen(str) != VIR_MAC_HEXLEN)
+        return -1;
+
+    size_t iaddr;
+    size_t istr;
+
+
+    for (istr = 0, iaddr = 0; iaddr < VIR_MAC_BUFLEN; istr += 2, iaddr++) {
+        unsigned int hex;
+
+        if (sscanf(&str[istr], "%02x", &hex) != 1)
+            break;

I'm not keen on sscanf(), but since we're already using it elsewhere,
and the existing virMacAddrParse uses the normally-blacklisted strtoul()
(and using virStrToLong_ui() on pieces of the string would require a
terminating character after each piece), I don't have a ready answer for
an alternative.

Why not just open-code it?  It's just two input bytes per iteration, and
we already have a helper function that makes it a very short function.
And strspn makes it easy to do input validation before starting the loop:

int
virMacAddrParseHex(const char *str, virMacAddrPtr addr)
{
    size_t i;

    if (strspn(str, "0123456789abcdefABCDEF") != VIR_MAC_HEXLEN ||
        str[VIR_MAC_HEXLEN])
        return -1;

    for (i = 0; i < VIR_MAC_BUFLEN; i++)
        addr->addr[i] = (virHexToBin(str[2 * i]) << 4 |
                         virHexToBin(str[2 * i + 1]));
    return 0;
}

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list