Matthias Bolte <matthias.bolte(a)googlemail.com> wrote on 04/14/2010
08:20:22 PM:
"Daniel P. Berrange", libvir-list
2010/4/15 Stefan Berger <stefanb(a)us.ibm.com>:
>
> libvir-list-bounces(a)redhat.com wrote on 04/14/2010 01:40:17 PM:
>
>
>> Please respond to "Daniel P. Berrange"
>>
>> On Wed, Apr 14, 2010 at 06:02:32PM +0200, Jim Meyering wrote:
>> > From: Jim Meyering <meyering(a)redhat.com>
>> >
>> > * src/nwfilter/nwfilter_ebiptables_driver.c
(ebiptablesApplyNewRules):
>> > Don't dereference a NULL or uninitialized pointer
when given
>> > an empty list of rules. Add an sa_assert(inst) in each loop to
>> > tell clang that the uses of "inst[i]" are valid.
>> > ---
>> > src/nwfilter/nwfilter_ebiptables_driver.c | 8 +++++---
>> > 1 files changed, 5 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/
>> nwfilter/nwfilter_ebiptables_driver.c
>> > index b481b4c..f54099f 100644
>> > --- a/src/nwfilter/nwfilter_ebiptables_driver.c
>> > +++ b/src/nwfilter/nwfilter_ebiptables_driver.c
>> > @@ -2834,11 +2834,11 @@ ebiptablesApplyNewRules(virConnectPtr conn
>> ATTRIBUTE_UNUSED,
>> > bool haveIptables = false;
>> > bool haveIp6tables = false;
>> >
>> > - if (inst)
>> > - qsort(inst, nruleInstances, sizeof(inst[0]),
>> > - ebiptablesRuleOrderSort);
>> > + if (nruleInstances > 1 && inst)
>> > + qsort(inst, nruleInstances, sizeof(inst[0]),
>> ebiptablesRuleOrderSort);
>> >
>> > for (i = 0; i < nruleInstances; i++) {
>> > + sa_assert (inst);
>> > if (inst[i]->ruleType == RT_EBTABLES) {
>> > if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP)
>> > chains_in |= (1 <<
inst[i]->neededProtocolChain);
>> > @@ -2881,6 +2881,7 @@ ebiptablesApplyNewRules(virConnectPtr conn
>> ATTRIBUTE_UNUSED,
>> > goto tear_down_tmpebchains;
>> >
>> > for (i = 0; i < nruleInstances; i++)
>> > + sa_assert (inst);
>
>
> Due to this statement here I get segmentation faults for which there
is no
> reason. I have no idea why that is but I have to deactivate this
line
for it
> to work again.
> The same is not true for the statement further above... So strange.
>
> Stefan
>
This one is obvious. The second for loop has no {} abound it's block.
Before the addition of sa_assert the switch formed the block of the
for loop and everything works as expected. Now the sa_assert line is
block of the for loop and the switch is no longer inside the loop.
Adding proper {} to the second for loop will fix the problem.
It has python-indentation :-) I didn't see it... and I'll fix this.
Stefan
Matthias