On 08/09/2012 04:08 PM, Eric Blake wrote:
On 08/09/2012 12:30 AM, Laine Stump wrote:
The meat of this patch is just moving the calls to virNWFilterRegisterCallbackDriver from each hypervisor's "register" function into its "initialize" function. The rest is just code movement to allow that, and a new virNWFilterUnRegisterCallbackDriver function to undo what the register function does.
The long explanation: <snip> but certainly helpful.
+++ b/src/conf/nwfilter_conf.c @@ -2829,6 +2829,24 @@ virNWFilterRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) }
void +virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) +{ + int i = 0; + + while (i < nCallbackDriver && callbackDrvArray[i] != cbd) + i++; + + if (i < nCallbackDriver) { + while (i < nCallbackDriver - 1) { + callbackDrvArray[i] = callbackDrvArray[i+1]; + i++; + } Is memmove() better than an open-coded loop?
Okay. it's not going to make any practical difference, but just to give the appearance of being optimized, I squashed in the following before pushing: diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 4fbbc78..8f8e053 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2837,10 +2837,8 @@ virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) i++; if (i < nCallbackDriver) { - while (i < nCallbackDriver - 1) { - callbackDrvArray[i] = callbackDrvArray[i+1]; - i++; - } + memmove(&callbackDrvArray[i], &callbackDrvArray[i+1], + (nCallbackDriver - i - 1) * sizeof(callbackDrvArray[i])); callbackDrvArray[i] = 0; nCallbackDriver--; }
+++ b/src/libvirt_private.syms @@ -880,6 +880,7 @@ virNWFilterRuleActionTypeToString; virNWFilterRuleDirectionTypeToString; virNWFilterRuleProtocolTypeToString; virNWFilterTestUnassignDef; +virNWFilterUnRegisterCallbackDriver; virNWFilterUnlockFilterUpdates; I don't know if we've been favoring case-sensitive ("C") or case-insensitive ("en_US.UTF-8") sorting in this file, so don't worry about whether you need to swap lines. I blame POSIX for introducing LC_COLLATE :)
I couldn't decide whether or not to be case sensitive when adding this entry, but the thought at the top of my mind was "I'm *sure* Eric will say something about this!" :-) Thanks for the review. I squashed in the bit above and pushed it.