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

Hiroyuki Kaguchi fj7025cf at aa.jp.fujitsu.com
Fri Mar 21 02:47:00 UTC 2008


Hi, Jim

Thank you very much for your comments.

Regards
Hiroyuki Kaguchi

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




More information about the libvir-list mailing list