On 12/03/2014 06:37 AM, Max Reitz wrote: > Refcounts can theoretically be of type uint64_t; in order to be able to > represent the full range, qcow2_get_refcount() cannot use a single > variable to represent both all refcount values and also keep some values > reserved for errors. > > One solution would be to add an Error pointer parameter to > qcow2_get_refcount(); however, no caller could (currently) pass that > error message, so it would have to be emitted immediately and be > passed to the next caller by returning -EIO or something similar. > Therefore, an Error parameter does not offer any advantages here. > > The solution applied by this patch is simpler to use. Because no caller > would be able to pass the error message, they would have to print it and > free it, whereas with this patch the caller only needs to pass the > returned integer (which is often a no-op from the code perspective, > because that integer will be stored in a variable "ret" which will be > returned by the fail path of many callers). > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > block/qcow2-cluster.c | 8 ++--- > block/qcow2-refcount.c | 79 > +++++++++++++++++++++++++++----------------------- > block/qcow2.h | 3 +- > 3 files changed, 49 insertions(+), 41 deletions(-) >
> @@ -1670,7 +1677,7 @@ static void compare_refcounts(BlockDriverState *bs, > BdrvCheckResult *res, > > if (num_fixed) { > ret = update_refcount(bs, i << s->cluster_bits, 1, > - refcount2 - refcount1, > + (int)refcount2 - (int)refcount1, The casts look a bit odd (uint16_t already naturally promotes to int for 32-bit int), but keep it, since we'll have to re-think this operation when a later patch widens to 64 bits anyways. Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature