[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PULL 20/37] qcow2: Give the refcount cach
From: |
Peter Maydell |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PULL 20/37] qcow2: Give the refcount cache the minimum possible size by default |
Date: |
Mon, 28 May 2018 14:49:07 +0100 |
On 28 May 2018 at 09:58, Alberto Garcia <address@hidden> wrote:
> On Mon 28 May 2018 10:38:55 AM CEST, Kevin Wolf wrote:
>>> > + if (!refcount_cache_size_set) {
>>> > + *refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE *
>>> > s->cluster_size;
>>>
>>> ...but in the else clause down here, we don't have the cast, and
>>> Coverity complains that we evaluate a 32-bit result and then put it
>>> in a 64-bit variable. Should this have the (uint64_t) cast as well ?
>
> I suppose that's not because we put a 32-bit result in a 64-bit
> variable, but because it could theoretically overflow (if
> s->cluster_size > INT32_MAX / 4).
Well, coverity warns because it's one of those things that
could be correct code, or could be the author tripping over
one of C's less-than-obvious traps. 32x32->32 multiplies are
just as susceptible to overflow, but the heuristic Coverity
uses is "calculated a 32-bit multiply result and put it in
a 64-bit variable", on the assumption that the type of the
destination implies that whatever you're calculating could
well be that big, and "truncate the result of my multiply
even though it would fit in the destination" is a bit
unexpected. Coverity's wrong quite a bit on this one, naturally,
but it's usually easier to shut it up on the wrong guesses
for the benefit of the times when it turns out to be right.
thanks
-- PMM
- [Qemu-block] [PULL 14/37] blockjob: Implement block_job_set_speed() centrally, (continued)
- [Qemu-block] [PULL 14/37] blockjob: Implement block_job_set_speed() centrally, Kevin Wolf, 2018/05/15
- [Qemu-block] [PULL 19/37] specs/qcow2: Clarify that compressed clusters have the COPIED bit reset, Kevin Wolf, 2018/05/15
- [Qemu-block] [PULL 13/37] blockjob: Move RateLimit to BlockJob, Kevin Wolf, 2018/05/15
- [Qemu-block] [PULL 12/37] blockjob: Wrappers for progress counter access, Kevin Wolf, 2018/05/15
- [Qemu-block] [PULL 17/37] iotests: Split 214 off of 122, Kevin Wolf, 2018/05/15
- [Qemu-block] [PULL 22/37] iotests: Add failure matching to common.qemu, Kevin Wolf, 2018/05/15
- [Qemu-block] [PULL 20/37] qcow2: Give the refcount cache the minimum possible size by default, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 21/37] docs: Document the new default sizes of the qcow2 caches, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 26/37] block: Add BDRV_REQ_WRITE_UNCHANGED flag, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 28/37] block/quorum: Support BDRV_REQ_WRITE_UNCHANGED, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 18/37] Fix error message about compressed clusters with OFLAG_COPIED, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 23/37] iotests: Skip 181 and 201 without userfaultfd, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 27/37] block: Set BDRV_REQ_WRITE_UNCHANGED for COR writes, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 25/37] block: BLK_PERM_WRITE includes ..._UNCHANGED, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 31/37] iotests: Copy 197 for COR filter driver, Kevin Wolf, 2018/05/15
[Qemu-block] [PULL 29/37] block: Support BDRV_REQ_WRITE_UNCHANGED in filters, Kevin Wolf, 2018/05/15