Re: [Qemu-devel] [PATCH] migration: Appease coverity, skip empty block trees

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1486 bytes --]

On Fri, Jun 22, 2018 at 04:11:22PM -0400, John Snow wrote:
> If a tree consists exclusively of implicit filter nodes, we might crash
> QEMU. This configuration should not exist in practice, but if it did,
> skipping it would be fine.
> 
> For the purposes of debug builds, throw an assert to remind us that
> this configuration is truly unexpected, but if it's compiled out we
> will cope just fine.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  migration/block-dirty-bitmap.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index 3bafbbdc4c..02725293dd 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -287,6 +287,10 @@ static int init_dirty_bitmap_migration(void)
>          while (bs && bs->drv && bs->implicit) {
>              bs = backing_bs(bs);
>          }
> +        if (!bs) {
> +            g_assert_not_reached();
> +            continue;
> +        }

If bs can never be NULL, why test that it is non-NULL in the while loop
condition?

Try:

  /* Precondition: bs != NULL thanks to the for loop */
  while (bs->drv && bs->implicit) {
      bs = backing_bs(bs);
  }
  /* Postcondition: bs != NULL due to implicit node layout assumption */

Does this silence Coverity?  ISTR it looks for cues like the bs check in
the while loop condition to decide whether it's likely that a variable
could be NULL.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]