Am 25.09.2010 10:01, schrieb Blue Swirl:
On Thu, Sep 23, 2010 at 9:23 PM, Stefan Weil <w...@mail.berlios.de> wrote:
Am 23.09.2010 22:24, schrieb Blue Swirl:
On Thu, Sep 23, 2010 at 7:11 PM, Stefan Weil <w...@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<w...@mail.berlios.de>
wrote:
Adding the gcc format attribute detects a format bug
which is fixed here.
Cc: Blue Swirl<blauwir...@gmail.com>
Cc: Kevin Wolf<kw...@redhat.com>
Signed-off-by: Stefan Weil<w...@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.
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.
BDRV_SECTOR_MASK is the critical value. This should work:
#define BDRV_SECTOR_SIZE 512
#define BDRV_SECTOR_MASK (int64_t)(~(511ULL))
