| Message ID | c0d398ea9d7141bcbb9b3746f97852b4@h3c.com |
|---|---|
| State | New |
| Headers | show |
| Series | block: Fix integer promotion error in bdrv_getlength() | expand |
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
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
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
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
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
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; }
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(-) -- 2.7.4 Please ignore the [PATCH] block: Return the real error code in bdrv_getlength -- Best regards, Guoyi