| Message ID | 20211011155031.149158-1-hreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2: Silence clang -m32 compiler warning | expand |
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 >
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
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); /*
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(-)