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