[libvirt] [PATCH] nwfilter: fix crash during filter define when lxc driver failed startup

[Laine Stump]

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.