[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration: Appease coverity, skip empty block t
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH] migration: Appease coverity, skip empty block trees |
Date: |
Fri, 29 Jun 2018 17:36:25 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 06/29/2018 01:07 PM, Stefan Hajnoczi wrote:
> 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 <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;
>> + }
>
> 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.
>
I'll give this a go, but mechanically it looks suspect without an
assert(bs) in the loop body, but that would definitely silence Coverity.
--js