qemu-block
[Top][All Lists]
Advanced

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

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


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH] migration: Appease coverity, skip empty block trees
Date: Fri, 22 Jun 2018 17:24:07 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0

On 06/22/2018 05:11 PM, 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.

Well... with your explanation, your patch is correct.
But do we really want to maintain a 'debug with assert' vs 'production
without assertions' codebase?

$ git grep g_assert_not_reached | egrep -v '^tests/' | wc -l
406

Does Coverity require all these 406 lines to behave with a
"configuration [that] should not exist in practice"?

> 
> Signed-off-by: John Snow <address@hidden>
> ---
>  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;

Can we choose one or the other?

> +        }
>  
>          for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
>               bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]