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

Re: [libvirt] [PATCH 2/3] Move virMacAddrXXX functions to src/util/virmacaddr.[ch]



On 01/27/2012 10:37 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> Move the virMacAddrXXX functions out of util.[ch] and into a
> new dedicate file virmacaddr.[ch]
> ---
>  src/Makefile.am             |    1 +
>  src/conf/capabilities.h     |    2 +-
>  src/conf/network_conf.h     |    2 +-
>  src/libvirt_private.syms    |   11 ++-
>  src/util/util.c             |   99 ---------------------------------
>  src/util/util.h             |   13 ----
>  src/util/virmacaddr.c       |  128 +++++++++++++++++++++++++++++++++++++++++++
>  src/util/virmacaddr.h       |   41 ++++++++++++++
>  src/util/virnetdev.c        |    1 +
>  src/util/virnetdevmacvlan.c |    2 +-
>  10 files changed, 181 insertions(+), 119 deletions(-)

More insertions than deletions, but probably due to copyright headers :)

> +++ b/src/util/virmacaddr.c
> @@ -0,0 +1,128 @@
> +/*
> + * virmacaddr.c: MAC address handling
> + *
> + * Copyright (C) 2012 Red Hat, Inc.

When doing code motion, are we required to carry over the copyright
years from the earlier location of the code?

> +
> +/* Compare two MAC addresses, ignoring differences in case,
> + * as well as leading zeros.
> + */
> +int
> +virMacAddrCompare (const char *p, const char *q)

As long as you are moving things, would you like to fix spacing before '('?

> +{
> +    unsigned char c, d;
> +    do {
> +        while (*p == '0' && c_isxdigit (p[1]))
> +            ++p;
> +        while (*q == '0' && c_isxdigit (q[1]))

and here too?

> +            ++q;
> +        c = c_tolower (*p);
> +        d = c_tolower (*q);

and here

> +
> +        result = strtoul(str, &end_ptr, 16);
> +
> +        if ((end_ptr - str) < 1 || 2 < (end_ptr - str) ||
> +            (errno != 0) ||
> +            (0xFF < result))

That last check is impossible.  Yes, I know it's just code motion, and
the dead code was present in the original as well, but since we already
did a length bound of at most two hex characters, we know that the
result did not exceed 0xff.

Why are we even bothering with strtoul for a two-character conversion?
This seems like it is more efficient when open-coded.  But in the
interest of pure code motion, I'd do any cleanups as a separate patch.

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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