* [Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size
@ 2018-05-28 15:01 Alberto Garcia
2018-05-29 15:36 ` Eric Blake
2018-05-29 18:12 ` Kevin Wolf
0 siblings, 2 replies; 3+ messages in thread
From: Alberto Garcia @ 2018-05-28 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Alberto Garcia, qemu-block, Peter Maydell, Kevin Wolf
MIN_REFCOUNT_CACHE_SIZE is 4 and the cluster size is guaranteed to be
at most 2MB, so the minimum refcount cache size (in bytes) is always
going to fit in a 32-bit integer.
Coverity doesn't know that, and since we're storing the result in a
uint64_t (*refcount_cache_size) it thinks that we need the 64 bits and
that we probably want to do a 64-bit multiplication to prevent the
result from being truncated.
This is a false positive in this case, but it's a fair warning.
We could do a 64-bit multiplication to get rid of it, but since we
know that a 32-bit variable is enough to store this value let's simply
reuse min_refcount_cache, make it a normal int and stop doing casts.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
---
block/qcow2.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 6d532470a8..a007dc4246 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -768,6 +768,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
BDRVQcow2State *s = bs->opaque;
uint64_t combined_cache_size;
bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set;
+ int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size;
combined_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_CACHE_SIZE);
l2_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_L2_CACHE_SIZE);
@@ -804,8 +805,6 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
} else {
uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
- uint64_t min_refcount_cache =
- (uint64_t) MIN_REFCOUNT_CACHE_SIZE * s->cluster_size;
/* Assign as much memory as possible to the L2 cache, and
* use the remainder for the refcount cache */
@@ -825,7 +824,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
* s->cluster_size);
}
if (!refcount_cache_size_set) {
- *refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size;
+ *refcount_cache_size = min_refcount_cache;
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size
2018-05-28 15:01 [Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size Alberto Garcia
@ 2018-05-29 15:36 ` Eric Blake
2018-05-29 18:12 ` Kevin Wolf
1 sibling, 0 replies; 3+ messages in thread
From: Eric Blake @ 2018-05-29 15:36 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel; +Cc: Kevin Wolf, Peter Maydell, qemu-block
On 05/28/2018 10:01 AM, Alberto Garcia wrote:
> MIN_REFCOUNT_CACHE_SIZE is 4 and the cluster size is guaranteed to be
> at most 2MB, so the minimum refcount cache size (in bytes) is always
> going to fit in a 32-bit integer.
>
> Coverity doesn't know that, and since we're storing the result in a
> uint64_t (*refcount_cache_size) it thinks that we need the 64 bits and
> that we probably want to do a 64-bit multiplication to prevent the
> result from being truncated.
>
> This is a false positive in this case, but it's a fair warning.
> We could do a 64-bit multiplication to get rid of it, but since we
> know that a 32-bit variable is enough to store this value let's simply
> reuse min_refcount_cache, make it a normal int and stop doing casts.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> block/qcow2.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size
2018-05-28 15:01 [Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size Alberto Garcia
2018-05-29 15:36 ` Eric Blake
@ 2018-05-29 18:12 ` Kevin Wolf
1 sibling, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2018-05-29 18:12 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Peter Maydell
Am 28.05.2018 um 17:01 hat Alberto Garcia geschrieben:
> MIN_REFCOUNT_CACHE_SIZE is 4 and the cluster size is guaranteed to be
> at most 2MB, so the minimum refcount cache size (in bytes) is always
> going to fit in a 32-bit integer.
>
> Coverity doesn't know that, and since we're storing the result in a
> uint64_t (*refcount_cache_size) it thinks that we need the 64 bits and
> that we probably want to do a 64-bit multiplication to prevent the
> result from being truncated.
>
> This is a false positive in this case, but it's a fair warning.
> We could do a 64-bit multiplication to get rid of it, but since we
> know that a 32-bit variable is enough to store this value let's simply
> reuse min_refcount_cache, make it a normal int and stop doing casts.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-29 18:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-28 15:01 [Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size Alberto Garcia
2018-05-29 15:36 ` Eric Blake
2018-05-29 18:12 ` Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.