* RE: [PATCH] block: Return the real error code in bdrv_getlength
@ 2020-11-05 5:41 Tuguoyi
2020-11-05 13:13 ` Eric Blake
0 siblings, 1 reply; 4+ messages in thread
From: Tuguoyi @ 2020-11-05 5:41 UTC (permalink / raw)
To: Kevin Wolf, Max Reitz, qemu-block; +Cc: qemu-devel
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' <kwolf@redhat.com>; 'Max Reitz' <mreitz@redhat.com>;
> 'qemu-block@nongnu.org' <qemu-block@nongnu.org>
> Cc: 'qemu-devel@nongnu.org' <qemu-devel@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 <tu.guoyi@h3c.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: Return the real error code in bdrv_getlength
2020-11-05 5:41 [PATCH] block: Return the real error code in bdrv_getlength Tuguoyi
@ 2020-11-05 13:13 ` Eric Blake
0 siblings, 0 replies; 4+ messages in thread
From: Eric Blake @ 2020-11-05 13:13 UTC (permalink / raw)
To: Tuguoyi, Kevin Wolf, Max Reitz, qemu-block; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: Return the real error code in bdrv_getlength
2020-11-05 3:10 Tuguoyi
@ 2020-11-05 13:19 ` Eric Blake
0 siblings, 0 replies; 4+ messages in thread
From: Eric Blake @ 2020-11-05 13:19 UTC (permalink / raw)
To: Tuguoyi, Kevin Wolf, Max Reitz, qemu-block; +Cc: qemu-devel
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 <tu.guoyi@h3c.com>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] block: Return the real error code in bdrv_getlength
@ 2020-11-05 3:10 Tuguoyi
2020-11-05 13:19 ` Eric Blake
0 siblings, 1 reply; 4+ messages in thread
From: Tuguoyi @ 2020-11-05 3:10 UTC (permalink / raw)
To: Kevin Wolf, Max Reitz, qemu-block; +Cc: qemu-devel
The return code from bdrv_nb_sectors() should be checked before doing
the following sanity check.
Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-05 13:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05 5:41 [PATCH] block: Return the real error code in bdrv_getlength Tuguoyi
2020-11-05 13:13 ` Eric Blake
-- strict thread matches above, loose matches on Subject: below --
2020-11-05 3:10 Tuguoyi
2020-11-05 13:19 ` Eric Blake
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).