On 11/4/20 11:41 PM, Tuguoyi wrote:
> Sorry, please ignore this patch, it's not a right fix
What's not right about it?
>> +++ b/block.c
>> @@ -5082,6 +5082,10 @@ int64_t bdrv_getlength(BlockDriverState *bs)
>> {
>> int64_t ret = bdrv_nb_sectors(bs);
>>
>> + if (ret < 0) {
>> + return ret;
>> + }
>> +
>> ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret;
You are correct that the old code was broken (since
INT64_MAX/BDRV_SECTOR_SIZE is unsigned, any negative ret is likewise
promoted to unsigned and we mistakenly return -EFBIG instead of the
original error). But your new code works just fine: because we
guarantee with the early return above that ret is a positive value, it
doesn't matter that the rest of this ?: is performed in unsigned math.
>> return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
Of course, having two ?: in a row is odd; we could instead write:
if (ret < 0){
return ret;
}
if (ret > INT64_MAX / BDRV_SECTOR_SIZE) {
return -EFBIG;
}
return ret * BDRV_SECTOR_SIZE;
for the same result.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
