Re: [Qemu-devel] [PATCH v12 01/10] qcow2: Use consistent switch indentation

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 6671 bytes --]

On 04.05.2017 05:07, Eric Blake wrote:
> Fix a couple of inconsistent indentations, before an upcoming
> patch further tweaks the switch statements.  While at it, make
> some tweaks for shorter lines to keep checkpatch happy (best
> viewed with 'git diff -b').
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v12: new patch
> ---
>  block/qcow2-cluster.c  | 32 ++++++++---------
>  block/qcow2-refcount.c | 96 ++++++++++++++++++++++++++------------------------
>  2 files changed, 65 insertions(+), 63 deletions(-)

[...]

> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 4efca7e..a5a0076 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1117,70 +1117,72 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
>                  goto fail;
>              }
> 
> -            for(j = 0; j < s->l2_size; j++) {
> +            for (j = 0; j < s->l2_size; j++) {
>                  uint64_t cluster_index;
> +                uint64_t offset_masked;
> 
>                  offset = be64_to_cpu(l2_table[j]);
>                  old_offset = offset;
> +                offset_masked = offset & L2E_OFFSET_MASK;

I'd rather rename "offset" to "entry" (or "l2_entry") and name this just
"offset". Much less confusing.

Also, I really wouldn't mind splitting the non-indentation changes off
into their own patch.

Tentative

Reviewed-by: Max Reitz <mreitz@redhat.com>

>                  offset &= ~QCOW_OFLAG_COPIED;
> 
>                  switch (qcow2_get_cluster_type(offset)) {
> -                    case QCOW2_CLUSTER_COMPRESSED:
> -                        nb_csectors = ((offset >> s->csize_shift) &
> -                                       s->csize_mask) + 1;
> -                        if (addend != 0) {
> -                            ret = update_refcount(bs,
> -                                (offset & s->cluster_offset_mask) & ~511,
> -                                nb_csectors * 512, abs(addend), addend < 0,
> -                                QCOW2_DISCARD_SNAPSHOT);
> -                            if (ret < 0) {
> -                                goto fail;
> -                            }
> -                        }
> -                        /* compressed clusters are never modified */
> -                        refcount = 2;
> -                        break;
> -
> -                    case QCOW2_CLUSTER_NORMAL:
> -                    case QCOW2_CLUSTER_ZERO:
> -                        if (offset_into_cluster(s, offset & L2E_OFFSET_MASK)) {
> -                            qcow2_signal_corruption(bs, true, -1, -1, "Data "
> -                                                    "cluster offset %#llx "
> -                                                    "unaligned (L2 offset: %#"
> -                                                    PRIx64 ", L2 index: %#x)",
> -                                                    offset & L2E_OFFSET_MASK,
> -                                                    l2_offset, j);
> -                            ret = -EIO;
> +                case QCOW2_CLUSTER_COMPRESSED:
> +                    nb_csectors = ((offset >> s->csize_shift) &
> +                                   s->csize_mask) + 1;
> +                    if (addend != 0) {
> +                        ret = update_refcount(bs,
> +                            (offset & s->cluster_offset_mask) & ~511,
> +                            nb_csectors * 512, abs(addend), addend < 0,
> +                            QCOW2_DISCARD_SNAPSHOT);
> +                        if (ret < 0) {
>                              goto fail;
>                          }
> +                    }
> +                    /* compressed clusters are never modified */
> +                    refcount = 2;
> +                    break;
> 
> -                        cluster_index = (offset & L2E_OFFSET_MASK) >> s->cluster_bits;
> -                        if (!cluster_index) {
> -                            /* unallocated */
> -                            refcount = 0;
> -                            break;
> -                        }
> -                        if (addend != 0) {
> -                            ret = qcow2_update_cluster_refcount(bs,
> +                case QCOW2_CLUSTER_NORMAL:
> +                case QCOW2_CLUSTER_ZERO:
> +                    if (offset_into_cluster(s, offset_masked)) {
> +                        qcow2_signal_corruption(bs, true, -1, -1, "Data "
> +                                                "cluster offset %#" PRIx64
> +                                                " unaligned (L2 offset: %#"
> +                                                PRIx64 ", L2 index: %#x)",
> +                                                offset_masked,
> +                                                l2_offset, j);
> +                        ret = -EIO;
> +                        goto fail;
> +                    }
> +
> +                    cluster_index = offset_masked >> s->cluster_bits;
> +                    if (!cluster_index) {
> +                        /* unallocated */
> +                        refcount = 0;
> +                        break;
> +                    }
> +                    if (addend != 0) {
> +                        ret = qcow2_update_cluster_refcount(bs,
>                                      cluster_index, abs(addend), addend < 0,
>                                      QCOW2_DISCARD_SNAPSHOT);
> -                            if (ret < 0) {
> -                                goto fail;
> -                            }
> -                        }
> -
> -                        ret = qcow2_get_refcount(bs, cluster_index, &refcount);
>                          if (ret < 0) {
>                              goto fail;
>                          }
> -                        break;
> +                    }
> 
> -                    case QCOW2_CLUSTER_UNALLOCATED:
> -                        refcount = 0;
> -                        break;
> +                    ret = qcow2_get_refcount(bs, cluster_index, &refcount);
> +                    if (ret < 0) {
> +                        goto fail;
> +                    }
> +                    break;
> 
> -                    default:
> -                        abort();
> +                case QCOW2_CLUSTER_UNALLOCATED:
> +                    refcount = 0;
> +                    break;
> +
> +                default:
> +                    abort();
>                  }
> 
>                  if (refcount == 1) {
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]