On 2017-11-14 16:02, Alberto Garcia wrote:
> On Fri 10 Nov 2017 09:31:10 PM CET, Max Reitz wrote:
>> +static int64_t get_refblock_offset(BlockDriverState *bs, uint64_t offset)
>> +{
>> +    BDRVQcow2State *s = bs->opaque;
>> +    uint32_t index = offset_to_reftable_index(s, offset);
>> +    int64_t covering_refblock_offset = 0;
>> +
>> +    if (index < s->refcount_table_size) {
>> +        covering_refblock_offset = s->refcount_table[index] & REFT_OFFSET_MASK;
>> +    }
>> +    if (!covering_refblock_offset) {
>> +        qcow2_signal_corruption(bs, true, -1, -1, "Refblock at %#" PRIx64 " is "
>> +                                "not covered by the refcount structures",
>> +                                offset);
>> +        return -EIO;
>> +    }
>> +
>> +    return covering_refblock_offset;
>> +}
> 
> Isn't it simpler to do something like this instead?
> 
>    if (index >= s->refcount_table_size) {
>        qcow2_signal_corruption(...);
>        return -EIO;
>    }
>    return s->refcount_table[index] & REFT_OFFSET_MASK;

But that doesn't cover the case were s->refcount_table[index] &
REFT_OFFSET_MASK is 0.

Max