16.11.2018 21:43, John Snow wrote: > Coverity warns that backing_bs() could give us a NULL pointer, which > we then use without checking that it isn't. > > In our loop condition, we check bs && bs->drv as a point of habit, but > by nature of the block graph, we cannot have null bs pointers here. > > This loop skips only implicit nodes, which always have children, so > this loop should never encounter a null value.
You mean, always have backing (not file for ex.)? Should we at least add a comment near "bool implicit;" that the node must have backing.. Do we have filters, using 'file' child instead of backing, will we want to auto insert them, and therefore mark them with implicit=true? And one more thing: So, it's looks like a wrong way to search for all block-nodes, instead of looping through backing chain to the first not-implicit bds, we must recursively explore the whole block graph, to find _all_ the bitmaps. > > Tighten the loop condition to coax Coverity into dropping > its false positive. > > Suggested-by: Stefan Hajnoczi <stefa...@redhat.com> > Signed-off-by: John Snow <js...@redhat.com> > --- > migration/block-dirty-bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c > index 5e90f44c2f..00c068fda3 100644 > --- a/migration/block-dirty-bitmap.c > +++ b/migration/block-dirty-bitmap.c > @@ -284,7 +284,7 @@ static int init_dirty_bitmap_migration(void) > const char *drive_name = bdrv_get_device_or_node_name(bs); > > /* skip automatically inserted nodes */ > - while (bs && bs->drv && bs->implicit) { > + while (bs->drv && bs->implicit) { > bs = backing_bs(bs); > } > > -- Best regards, Vladimir