Re: [Qemu-devel] [PATCH v13 05/12] qcow2: Name typedef for cluster type

[Max Reitz]

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 <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
> 
> ---
> 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.

> @@ -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.

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 :-))

Max

signature.asc
Description: OpenPGP digital signature