[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/7] nwfilter_ebiptables_driver.c: avoid NULL dereference



Stefan Berger wrote:
>> If it's not 0, then you must have one of these two envvars set:
>>
>>   test -n "$CCC_ANALYZER_ANALYSIS$COVERITY_BUILD_COMMAND" && echo oops
>
> got 'oops' here.

That is surprising (esp. since your definition of STATIC_ANALYSIS was 0).
You get that on the command line?
Which of those two variables is defined for you?
And to what?

  echo "$CCC_ANALYZER_ANALYSIS"
  echo "$COVERITY_BUILD_COMMAND"

If you confirm, I may have to adjust the configure-time
test to be less prone to false positive.

>> How is sa_assert defined for you?
>>
>>     $ grep -C3 sa_assert src/internal.h
>>     # if STATIC_ANALYSIS
>>     #  undef NDEBUG /* Don't let a prior NDEBUG definition cause trouble.  */
>>     #  include <assert.h>
>>     #  define sa_assert(expr) assert (expr)
>>     # else
>>     #  define sa_assert(expr) /* empty */
>>     # endif
>>
>> With those, the net result in your file should be that
>> sa_assert is a no-op.
>
> Yes, I agree. My understanding also.
>
>> If you're still convinced that the segfault is due to that use
>> of sa_assert, please send me preprocessed output for that file, i.e.,
>>
>>     cd src
>>     f=nwfilter_ebiptables_driver
>>     touch nwfilter/$f.c
>>     la=libvirt_driver_nwfilter_la
>>     lo=$la-$f.lo
>>     make AM_CPPFLAGS='-E -dD' $lo
>>     mv .libs/$la-$f.o $f.i
>>
>> The cpp-preprocessed output is now in
>>
>>     src/nwfilter_ebiptables_driver.i
>>
>> You should be able to see that sa_assert expands to nothing:
>>
>>     $ grep sa_assert $f.i
>>     #define sa_assert(expr)
>
> Well, not quite true. I see the lonely semicolon there that's the remainder from
> sa_assert(); -> one would have to write sa_assert(), which isn't nice, either...
>
>    if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
>        goto tear_down_tmpebchains;
>
>    for (i = 0; i < nruleInstances; i++)
>        ;
>        switch (inst[i]->ruleType) {

Right.  It's obvious, once you notice the missing curly brace.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]