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.