|
From: | Stefan Weil |
Subject: | [Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error |
Date: | Wed, 13 Oct 2010 21:06:22 +0200 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100913 Iceowl/1.0b1 Icedove/3.0.7 |
Am 25.09.2010 10:01, schrieb Blue Swirl:
On Thu, Sep 23, 2010 at 9:23 PM, Stefan Weil<address@hidden> wrote:Am 23.09.2010 22:24, schrieb 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.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.Or (3) Use PRId64, change blkverify_iovec_compare, leave BDRV_SECTOR_SIZE unchanged but add a cast to int64_t here. Grepping for BDRV_SECTOR_SIZE shows that it is used in several places in size_t or off_t expressions, so long long is as good as any other large type. I think Kevin should decide.
Kevin, how should this get fixed? I suggest committing my last patch version sent on 2010-09-24 ("[PATCH] block: Use GCC_FMT_ATTR and fix a format error"), but I don't mind if you have a different solution. Regards, Stefan
[Prev in Thread] | Current Thread | [Next in Thread] |