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