On 05/08/2017 10:43 AM, Max Reitz wrote: > On 07.05.2017 02:05, Eric Blake wrote: >> Although it doesn't add all that much type safety (this is C, after >> all), it does add a bit of legibility to use the name QCow2ClusterType >> instead of a plain int. >> >> In particular, qcow2_get_cluster_offset() has an overloaded return >> type; a QCow2ClusterType on success, and -errno on failure; keeping >> the cluster type in a separate variable makes it slightly easier for >> the next patch to make further computations based on the type. >> >> Suggested-by: Max Reitz <mre...@redhat.com> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> >> --- >> v13: new patch >> --- >> block/qcow2.h | 6 +++--- >> block/qcow2-cluster.c | 17 +++++++++-------- >> block/qcow2-refcount.c | 2 +- >> 3 files changed, 13 insertions(+), 12 deletions(-) > > [...] > >> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c >> index f3bfce6..ed78a30 100644 >> --- a/block/qcow2-cluster.c >> +++ b/block/qcow2-cluster.c > > Above this hunk, there is count_contiguous_clusters() with a variable > "first_cluster_type" that could be a QCow2ClusterType as well.
Hmm, I guess I didn't catch them all. Yes, that one definitely qualified. > >> @@ -340,7 +340,7 @@ static int count_contiguous_clusters(int nb_clusters, >> int cluster_size, >> */ >> static int count_contiguous_clusters_unallocated(int nb_clusters, >> uint64_t *l2_table, >> - int wanted_type) >> + QCow2ClusterType >> wanted_type) >> { >> int i; >> > > And some lines below this (in this function), there is a "type" variable > that (c|sh)ould be a QCow2ClusterType, too. That one I blame on poor rebasing on my side. > > Although it's quite a functional change, I would be willing to change > both when applying, if you allowed me to. > > (Once again, "no good deed shall go unpunished", as Markus likes to say :-)) Yes, both changes are appropriate, and I'm fine with you making the tweaks as part of putting it on your tree (and the corresponding fallout on 6/12). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature