Patchwork block: fix a warning and possible truncation

login
register
mail settings
Submitter Blue Swirl
Date June 14, 2010, 6:55 p.m.
Message ID <AANLkTilSuir7Sd-ee_HCvsViF9xrHgcohL0cVV1gZLak@mail.gmail.com>
Download mbox | patch
Permalink /patch/55573/
State New
Headers show

Comments

Blue Swirl - June 14, 2010, 6:55 p.m.
Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
/src/qemu/block.c: In function `bdrv_info_stats_bs':
/src/qemu/block.c:1548: warning: long long int format, long unsigned
int arg (arg 6)

There may be also truncation effects.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
Alternatively 'ULL' prefix could be appended to BDRV_SECTOR_SIZE
definition but that may have other side effects.

 block.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Kevin Wolf - June 15, 2010, 7:52 a.m.
Am 14.06.2010 20:55, schrieb Blue Swirl:
> Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
> /src/qemu/block.c: In function `bdrv_info_stats_bs':
> /src/qemu/block.c:1548: warning: long long int format, long unsigned
> int arg (arg 6)
> 
> There may be also truncation effects.
> 
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

Thanks, applied to the block branch.

But why is this even needed? wr_highest_sector is already uint64_t, so
wouldn't you expect the result to be uint64_t, too?

Kevin
Markus Armbruster - June 15, 2010, 8:08 a.m.
Kevin Wolf <kwolf@redhat.com> writes:

> Am 14.06.2010 20:55, schrieb Blue Swirl:
>> Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
>> /src/qemu/block.c: In function `bdrv_info_stats_bs':
>> /src/qemu/block.c:1548: warning: long long int format, long unsigned
>> int arg (arg 6)
>> 
>> There may be also truncation effects.
>> 
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>
> Thanks, applied to the block branch.
>
> But why is this even needed? wr_highest_sector is already uint64_t, so
> wouldn't you expect the result to be uint64_t, too?

Makes me wonder.  To what's uint64_t typedef'ed on this machine?  And to
what does PRId64 expand?
Alexander Graf - June 15, 2010, 9:52 a.m.
Am 14.06.2010 um 20:55 schrieb Blue Swirl <blauwirbel@gmail.com>:

> Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
> /src/qemu/block.c: In function `bdrv_info_stats_bs':
> /src/qemu/block.c:1548: warning: long long int format, long unsigned
> int arg (arg 6)
>
> There may be also truncation effects.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
> Alternatively 'ULL' prefix could be appended to BDRV_SECTOR_SIZE
> definition but that may have other side effects.

... Which are probably wanted. If there are more truncations, we want  
to catch them early, no?

Alex

>
Kevin Wolf - June 15, 2010, 9:59 a.m.
Am 15.06.2010 11:52, schrieb Alexander Graf:
> 
> Am 14.06.2010 um 20:55 schrieb Blue Swirl <blauwirbel@gmail.com>:
> 
>> Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
>> /src/qemu/block.c: In function `bdrv_info_stats_bs':
>> /src/qemu/block.c:1548: warning: long long int format, long unsigned
>> int arg (arg 6)
>>
>> There may be also truncation effects.
>>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>> Alternatively 'ULL' prefix could be appended to BDRV_SECTOR_SIZE
>> definition but that may have other side effects.
> 
> ... Which are probably wanted. If there are more truncations, we want  
> to catch them early, no?

Actually, it's there:

#define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)

That compiler warning doesn't make any sense to me.

Kevin

Patch

diff --git a/block.c b/block.c
index cacf11b..a7ab0b4 100644
--- a/block.c
+++ b/block.c
@@ -1545,7 +1545,8 @@  static QObject* bdrv_info_stats_bs(BlockDriverState *bs)
                              "} }",
                              bs->rd_bytes, bs->wr_bytes,
                              bs->rd_ops, bs->wr_ops,
-                             bs->wr_highest_sector * (long)BDRV_SECTOR_SIZE);
+                             bs->wr_highest_sector *
+                             (uint64_t)BDRV_SECTOR_SIZE);
     dict  = qobject_to_qdict(res);

     if (*bs->device_name) {