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

[Blue Swirl]

On Thu, Sep 23, 2010 at 7:11 PM, Stefan Weil <address@hidden> 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<address@hidden>
>>>  wrote:
>>>>
>>>> Adding the gcc format attribute detects a format bug
>>>> which is fixed here.
>>>>
>>>> Cc: Blue Swirl<address@hidden>
>>>> Cc: Kevin Wolf<address@hidden>
>>>> Signed-off-by: Stefan Weil<address@hidden>
>>>> ---
>>>>  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.