On 16 November 2018 at 16:16, John Snow <js...@redhat.com> wrote: > On 11/16/18 5:04 AM, Peter Maydell wrote: >> Personally I think the main benefit of this sort of Coverity >> warning is that it nudges you to work through and figure out >> whether something really can be NULL or not. Stefan's fix >> will satisfy Coverity, because what it's complaining about >> is that the code in one place assumes a pointer can't be NULL >> but in another place does have handling for NULL: it is the >> inconsistency that triggers the warning. If backing_bs() >> can't return NULL (ie if you would be happy in theory to put >> an assert() in after the call) then I think Stefan's fix is >> better. If it can return NULL ever then Vladimir's fix is >> what you want.
> I really don't think it can, but I don't actually know how to verify it > or to convince Coverity of the same. Stefan's suggestion seems most > appropriate if it actually does calm Coverity down. I think it will quieten Coverity; if it doesn't, then at that point we can happily mark the issue a false-positive. But as I say what Coverity is really identifying here is "you checked whether this pointer was NULL, but then there's a code path forward to a place where you used it without checking" -- so removing the unnecessary check will make it happy. > (Is it mechanically possible to violate this? That's very hard to audit > and promise for you. There are always bugs lurking somewhere else.) If you believe by design that it can't be NULL, then we're OK: if it ever turns out that it isn't, then we will get a nice easy-to-debug SEGV when we dereference the NULL pointer, and we can find out then what bug resulted in our design assumption being broken. > Stefan's suggestion is probably most appropriate, *if* it actually > shushes Coverity. I'll send you a small patch and you can find out? I > don't want to task offload but I also genuinely don't know if that will > hint to coverity strongly enough that this is a false positive. Coverity runs only on git master, so fixes have to go into master to be tested, unfortunately. thanks -- PMM
