Re: [PATCH] block: Return the real error code in bdrv_getlength
On 11/4/20 9:10 PM, Tuguoyi wrote: > The return code from bdrv_nb_sectors() should be checked before doing > the following sanity check. > > Signed-off-by: Guoyi Tu It looks like you sent several variations on this patch. A meta-observation: your mailer is attributing the patch to the spelling "Tuguoyi", while your S-o-b line is spelling "Guoyi Tu". It's worth being consistent between the two. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH] block: Return the real error code in bdrv_getlength
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
RE: [PATCH] block: Return the real error code in bdrv_getlength
Sorry, please ignore this patch, it's not a right fix -- Best regards, Guoyi > -Original Message- > From: tuguoyi (Cloud) > Sent: Thursday, November 05, 2020 11:11 AM > To: 'Kevin Wolf' ; 'Max Reitz' ; > 'qemu-block@nongnu.org' > Cc: 'qemu-de...@nongnu.org' > Subject: [PATCH] block: Return the real error code in bdrv_getlength > > The return code from bdrv_nb_sectors() should be checked before doing > the following sanity check. > > Signed-off-by: Guoyi Tu > --- > block.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/block.c b/block.c > index 430edf7..19ebbc0 100644 > --- a/block.c > +++ 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; > return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE; > } > -- > 2.7.4 > > > -- > Best regards, > Guoyi >