[PATCH] block: Fix integer promotion error in bdrv_getlength()

Tuguoyi posted 1 patch 5 years ago
Failed in applying to current master (apply log)
There is a newer version of this series
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] block: Fix integer promotion error in bdrv_getlength()
Posted by Tuguoyi 5 years ago
As BDRV_SECTOR_SIZE is of type uint64_t, the expression will
automatically convert the @ret to uint64_t. When an error code
returned from bdrv_nb_sectors(), the promoted @ret will be a very
large number, as a result the -EFBIG will be returned which is not the
real error code.

Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 430edf7..f14254c 100644
--- a/block.c
+++ b/block.c
@@ -5082,7 +5082,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
 {
     int64_t ret = bdrv_nb_sectors(bs);
 
-    ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret;
+    ret = ret > INT64_MAX / (int64_t)BDRV_SECTOR_SIZE ? -EFBIG : ret;
     return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
 }
 
-- 
2.7.4

Please ignore the [PATCH] block: Return the real error code in bdrv_getlength

--
Best regards,
Guoyi
Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
Posted by Max Reitz 5 years ago
On 05.11.20 06:40, Tuguoyi wrote:
> As BDRV_SECTOR_SIZE is of type uint64_t, the expression will
> automatically convert the @ret to uint64_t. When an error code
> returned from bdrv_nb_sectors(), the promoted @ret will be a very
> large number, as a result the -EFBIG will be returned which is not the
> real error code.
> 
> Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
Posted by Eric Blake 5 years ago
On 11/5/20 2:31 AM, Max Reitz wrote:
> On 05.11.20 06:40, Tuguoyi wrote:
>> As BDRV_SECTOR_SIZE is of type uint64_t, the expression will
>> automatically convert the @ret to uint64_t. When an error code
>> returned from bdrv_nb_sectors(), the promoted @ret will be a very
>> large number, as a result the -EFBIG will be returned which is not the
>> real error code.
>>
>> Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
>> ---
>>   block.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks, applied to my block branch:
> 
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> 

I actually preferred the v1 solution, rather than this v2, as it avoided
a cast.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
Posted by Max Reitz 5 years ago
On 05.11.20 14:14, Eric Blake wrote:
> On 11/5/20 2:31 AM, Max Reitz wrote:
>> On 05.11.20 06:40, Tuguoyi wrote:
>>> As BDRV_SECTOR_SIZE is of type uint64_t, the expression will
>>> automatically convert the @ret to uint64_t. When an error code
>>> returned from bdrv_nb_sectors(), the promoted @ret will be a very
>>> large number, as a result the -EFBIG will be returned which is not the
>>> real error code.
>>>
>>> Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
>>> ---
>>>    block.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Thanks, applied to my block branch:
>>
>> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
>>
> 
> I actually preferred the v1 solution, rather than this v2, as it avoided
> a cast.

I don’t, because it doesn’t make the ?: go away, so I prefer the less 
invasive version.

If you want to send your suggested version (that drops both ?:), I’m 
happy to take that instead.

Max


Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
Posted by Eric Blake 5 years ago
On 11/5/20 7:26 AM, Max Reitz wrote:

>> I actually preferred the v1 solution, rather than this v2, as it avoided
>> a cast.
> 
> I don’t, because it doesn’t make the ?: go away, so I prefer the less
> invasive version.
> 
> If you want to send your suggested version (that drops both ?:), I’m
> happy to take that instead.

Will do.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


RE: [PATCH] block: Fix integer promotion error in bdrv_getlength()
Posted by Tuguoyi 5 years ago
On Wed, 2019-07-03 at 10:13 -0500, Eric Blake wrote:
> On 11/5/20 2:31 AM, Max Reitz wrote:
> > On 05.11.20 06:40, Tuguoyi wrote:
> >> As BDRV_SECTOR_SIZE is of type uint64_t, the expression will
> >> automatically convert the @ret to uint64_t. When an error code
> >> returned from bdrv_nb_sectors(), the promoted @ret will be a very
> >> large number, as a result the -EFBIG will be returned which is not the
> >> real error code.
> >>
> >> Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
> >> ---
> >>   block.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Thanks, applied to my block branch:
> >
> > https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> >
> 
> I actually preferred the v1 solution, rather than this v2, as it avoided
> a cast.

There are several type promotion bugs(commits '570542ec') found recently, 
so i think explicitly casting the integer type can give a hint that there
is a potential type promotion if you don't do that.
 
Actually your solution look much simple and clear, and it's ok for me

--
Best regards,
Guoyi