[libvirt] [PATCH 1/2] util: Functions to update host network device's multicast filter

[Tony Krowiak]

On 10/08/2014 12:59 PM, Eric Blake wrote:
> On 10/08/2014 09:36 AM, Laine Stump wrote:
>> On 10/06/2014 05:37 PM, akrowiak at linux.vnet.ibm.com wrote:
>>> From: Tony Krowiak <akrowiak at 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 at 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 '*'.
I will fix that.
>
>>> +{
>>> +    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;
> }
Looks good, I'll put that in.
>
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141008/a96c2b4e/attachment-0001.htm>