qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] block: Fix integer promotion error in bdrv_getlength()
@ 2020-11-05  5:40 Tuguoyi
  2020-11-05  8:31 ` Max Reitz
  0 siblings, 1 reply; 6+ messages in thread
From: Tuguoyi @ 2020-11-05  5:40 UTC (permalink / raw)
  To: Kevin Wolf, Max Reitz, qemu-block@nongnu.org
  Cc: Chengchiwen, qemu-devel@nongnu.org, Wangyong

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
  2020-11-05  5:40 [PATCH] block: Fix integer promotion error in bdrv_getlength() Tuguoyi
@ 2020-11-05  8:31 ` Max Reitz
  2020-11-05 13:14   ` Eric Blake
  0 siblings, 1 reply; 6+ messages in thread
From: Max Reitz @ 2020-11-05  8:31 UTC (permalink / raw)
  To: Tuguoyi, Kevin Wolf, qemu-block@nongnu.org
  Cc: Chengchiwen, qemu-devel@nongnu.org, Wangyong

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
  2020-11-05  8:31 ` Max Reitz
@ 2020-11-05 13:14   ` Eric Blake
  2020-11-05 13:26     ` Max Reitz
  2020-11-06  2:06     ` Tuguoyi
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Blake @ 2020-11-05 13:14 UTC (permalink / raw)
  To: Max Reitz, Tuguoyi, Kevin Wolf, qemu-block@nongnu.org
  Cc: Chengchiwen, qemu-devel@nongnu.org, Wangyong

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
  2020-11-05 13:14   ` Eric Blake
@ 2020-11-05 13:26     ` Max Reitz
  2020-11-05 15:39       ` Eric Blake
  2020-11-06  2:06     ` Tuguoyi
  1 sibling, 1 reply; 6+ messages in thread
From: Max Reitz @ 2020-11-05 13:26 UTC (permalink / raw)
  To: Eric Blake, Tuguoyi, Kevin Wolf, qemu-block@nongnu.org
  Cc: Chengchiwen, qemu-devel@nongnu.org, Wangyong

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] block: Fix integer promotion error in bdrv_getlength()
  2020-11-05 13:26     ` Max Reitz
@ 2020-11-05 15:39       ` Eric Blake
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-11-05 15:39 UTC (permalink / raw)
  To: Max Reitz, Tuguoyi, Kevin Wolf, qemu-block@nongnu.org
  Cc: Chengchiwen, qemu-devel@nongnu.org, Wangyong

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] block: Fix integer promotion error in bdrv_getlength()
  2020-11-05 13:14   ` Eric Blake
  2020-11-05 13:26     ` Max Reitz
@ 2020-11-06  2:06     ` Tuguoyi
  1 sibling, 0 replies; 6+ messages in thread
From: Tuguoyi @ 2020-11-06  2:06 UTC (permalink / raw)
  To: Eric Blake, Max Reitz, Kevin Wolf, qemu-block@nongnu.org
  Cc: Chengchiwen, qemu-devel@nongnu.org, Wangyong

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-06  2:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-05  5:40 [PATCH] block: Fix integer promotion error in bdrv_getlength() Tuguoyi
2020-11-05  8:31 ` Max Reitz
2020-11-05 13:14   ` Eric Blake
2020-11-05 13:26     ` Max Reitz
2020-11-05 15:39       ` Eric Blake
2020-11-06  2:06     ` Tuguoyi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).