[Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Stefan Weil <weil@mail.berlios.de>]

Am 23.09.2010 22:24, schrieb Blue Swirl:
> On Thu, Sep 23, 2010 at 7:11 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>> Am 23.09.2010 21:03, schrieb Stefan Weil:
>>>
>>> Am 23.09.2010 20:53, schrieb Blue Swirl:
>>>>
>>>> On Thu, Sep 23, 2010 at 6:37 PM, Stefan Weil<weil@mail.berlios.de>
>>>>  wrote:
>>>>>
>>>>> Adding the gcc format attribute detects a format bug
>>>>> which is fixed here.
>>>>>
>>>>> Cc: Blue Swirl<blauwirbel@gmail.com>
>>>>> Cc: Kevin Wolf<kwolf@redhat.com>
>>>>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>>>>> ---
>>>>>  block/blkverify.c |    5 +++--
>>>>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/block/blkverify.c b/block/blkverify.c
>>>>> index 8083464..b39fb67 100644
>>>>> --- a/block/blkverify.c
>>>>> +++ b/block/blkverify.c
>>>>> @@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
>>>>>     .cancel             = blkverify_aio_cancel,
>>>>>  };
>>>>>
>>>>> -static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
>>>>> +static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
>>>>> +                                             const char *fmt, ...)
>>>>>  {
>>>>>     va_list ap;
>>>>>
>>>>> @@ -300,7 +301,7 @@ static void blkverify_verify_readv(BlkverifyAIOCB
>>>>> *acb)
>>>>>     ssize_t offset = 
>>>>> blkverify_iovec_compare(acb->qiov,&acb->raw_qiov);
>>>>>     if (offset != -1) {
>>>>>         blkverify_err(acb, "contents mismatch in sector %ld",
>>>>> -                      acb->sector_num + (offset / BDRV_SECTOR_SIZE));
>>>>> +                      (long)(acb->sector_num + (offset /
>>>>> BDRV_SECTOR_SIZE)));
>>>>
>>>> sector_num is int64_t, so the correct fix is to change '%ld' to '%"
>>>> PRId64'.
>>>>
>>>
>>> I noticed that, too. But offset is ssize_t.
>>> Can you always be sure that (int64_t + ssize_t) results in a int64_t?
>>> I don't think it's so easy.
>>
>> I think you are correct, the format should use PRId64.
>> The type cast is still necessary, but should cast to int64_t.
>> (needed when int64_t == long and ssize_t == long long).
>>
>> If you agree, I'll send a new patch.
>
> It's also possible to cast offset to int64_t. Or perhaps even the type
> of the return value of blkverify_iovec_compare should be changed to
> int64_t.

Unless BDRV_SECTOR_SIZE is changed, too, this would
still need a type cast. So we have two possible solutions:

(1) Use %lld (should work because BDRV_SECTOR_SIZE is unsigned long long).
(2) Use PRId64. This needs changes for BDRV_SECTOR_SIZE and 
blkverify_iovec_compare.

Any preferences? I tend to (2), but that change is less local.