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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list