[PATCH] qcow2: Silence clang -m32 compiler warning

[PATCH] qcow2: Silence clang -m32 compiler warning

[Hanna Reitz]

With -m32, size_t is generally only a uint32_t.  That makes clang
complain that in the assertion

  assert(qiov->size <= INT64_MAX);

the range of the type of qiov->size (size_t) is too small for any of its
values to ever exceed INT64_MAX.

Cast qiov->size to uint64_t to silence clang.

Fixes: f7ef38dd1310d7d9db76d0aa16899cbc5744f36d
       ("block: use int64_t instead of uint64_t in driver read
       handlers")
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
I don't know whether this is the best possible solution, or whether we
should care about this at all (I personally think it's basically just
wrong for clang to warn about always-true conditions in assertions), but
I thought I might as well just send this patch as the basis for a
discussion.
---
 block/qcow2-cluster.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5727f92dcb..21884a1ab9 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -513,7 +513,8 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
      */
     assert(src_cluster_offset <= INT64_MAX);
     assert(src_cluster_offset + offset_in_cluster <= INT64_MAX);
-    assert(qiov->size <= INT64_MAX);
+    /* Cast qiov->size to uint64_t to silence a compiler warning on -m32 */
+    assert((uint64_t)qiov->size <= INT64_MAX);
     bdrv_check_qiov_request(src_cluster_offset + offset_in_cluster, qiov->size,
                             qiov, 0, &error_abort);
     /*
-- 
2.31.1

Re: [PATCH] qcow2: Silence clang -m32 compiler warning

[Eric Blake]

On Mon, Oct 11, 2021 at 05:50:31PM +0200, Hanna Reitz wrote:
> With -m32, size_t is generally only a uint32_t.  That makes clang
> complain that in the assertion
> 
>   assert(qiov->size <= INT64_MAX);
> 
> the range of the type of qiov->size (size_t) is too small for any of its
> values to ever exceed INT64_MAX.

Yep, I'm not surprised that we hit that.

> 
> Cast qiov->size to uint64_t to silence clang.
> 
> Fixes: f7ef38dd1310d7d9db76d0aa16899cbc5744f36d
>        ("block: use int64_t instead of uint64_t in driver read
>        handlers")
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
> I don't know whether this is the best possible solution, or whether we
> should care about this at all (I personally think it's basically just
> wrong for clang to warn about always-true conditions in assertions), but
> I thought I might as well just send this patch as the basis for a
> discussion.

I agree that the compiler is overly noisy, but the fix is simple
enough that I'm fine with it as written.

Reviewed-by: Eric Blake <eblake@redhat.com>

Since the original went through my tree, I'm happy to take this one
through my NBD tree as well.

> ---
>  block/qcow2-cluster.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 5727f92dcb..21884a1ab9 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -513,7 +513,8 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
>       */
>      assert(src_cluster_offset <= INT64_MAX);
>      assert(src_cluster_offset + offset_in_cluster <= INT64_MAX);
> -    assert(qiov->size <= INT64_MAX);
> +    /* Cast qiov->size to uint64_t to silence a compiler warning on -m32 */
> +    assert((uint64_t)qiov->size <= INT64_MAX);
>      bdrv_check_qiov_request(src_cluster_offset + offset_in_cluster, qiov->size,
>                              qiov, 0, &error_abort);
>      /*
> -- 
> 2.31.1
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] qcow2: Silence clang -m32 compiler warning

[Hanna Reitz]

On 11.10.21 18:24, Eric Blake wrote:
> On Mon, Oct 11, 2021 at 05:50:31PM +0200, Hanna Reitz wrote:
>> With -m32, size_t is generally only a uint32_t.  That makes clang
>> complain that in the assertion
>>
>>    assert(qiov->size <= INT64_MAX);
>>
>> the range of the type of qiov->size (size_t) is too small for any of its
>> values to ever exceed INT64_MAX.
> Yep, I'm not surprised that we hit that.
>
>> Cast qiov->size to uint64_t to silence clang.
>>
>> Fixes: f7ef38dd1310d7d9db76d0aa16899cbc5744f36d
>>         ("block: use int64_t instead of uint64_t in driver read
>>         handlers")
>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>> ---
>> I don't know whether this is the best possible solution, or whether we
>> should care about this at all (I personally think it's basically just
>> wrong for clang to warn about always-true conditions in assertions), but
>> I thought I might as well just send this patch as the basis for a
>> discussion.
> I agree that the compiler is overly noisy, but the fix is simple
> enough that I'm fine with it as written.

Well, I just hope clang won’t become even more clever in the future and 
realize the cast has no real effect...

> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> Since the original went through my tree, I'm happy to take this one
> through my NBD tree as well.

Thanks!

Hanna