Re: [PATCH 01/10] qcow2-refcount: improve style of check_refcounts_l2()

2021-05-04 Thread Eric Blake
On 5/4/21 10:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>  - don't use same name for size in bytes and in entries
>  - use g_autofree for l2_table
>  - add whitespace
>  - fix block comment style
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-refcount.c | 47 +-
>  1 file changed, 24 insertions(+), 23 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




[PATCH 01/10] qcow2-refcount: improve style of check_refcounts_l2()

2021-05-04 Thread Vladimir Sementsov-Ogievskiy
 - don't use same name for size in bytes and in entries
 - use g_autofree for l2_table
 - add whitespace
 - fix block comment style

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-refcount.c | 47 +-
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 8e649b008e..2734338625 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1601,23 +1601,22 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
   int flags, BdrvCheckMode fix, bool active)
 {
 BDRVQcow2State *s = bs->opaque;
-uint64_t *l2_table, l2_entry;
+uint64_t l2_entry;
 uint64_t next_contiguous_offset = 0;
-int i, l2_size, nb_csectors, ret;
+int i, nb_csectors, ret;
+size_t l2_size_bytes = s->l2_size * l2_entry_size(s);
+g_autofree uint64_t *l2_table = g_malloc(l2_size_bytes);
 
 /* Read L2 table from disk */
-l2_size = s->l2_size * l2_entry_size(s);
-l2_table = g_malloc(l2_size);
-
-ret = bdrv_pread(bs->file, l2_offset, l2_table, l2_size);
+ret = bdrv_pread(bs->file, l2_offset, l2_table, l2_size_bytes);
 if (ret < 0) {
 fprintf(stderr, "ERROR: I/O error in check_refcounts_l2\n");
 res->check_errors++;
-goto fail;
+return ret;
 }
 
 /* Do the actual checks */
-for(i = 0; i < s->l2_size; i++) {
+for (i = 0; i < s->l2_size; i++) {
 l2_entry = get_l2_entry(s, l2_table, i);
 
 switch (qcow2_get_cluster_type(bs, l2_entry)) {
@@ -1647,14 +1646,15 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 l2_entry & QCOW2_COMPRESSED_SECTOR_MASK,
 nb_csectors * QCOW2_COMPRESSED_SECTOR_SIZE);
 if (ret < 0) {
-goto fail;
+return ret;
 }
 
 if (flags & CHECK_FRAG_INFO) {
 res->bfi.allocated_clusters++;
 res->bfi.compressed_clusters++;
 
-/* Compressed clusters are fragmented by nature.  Since they
+/*
+ * Compressed clusters are fragmented by nature.  Since they
  * take up sub-sector space but we only have sector granularity
  * I/O we need to re-read the same sectors even for adjacent
  * compressed clusters.
@@ -1700,9 +1700,11 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 if (ret < 0) {
 fprintf(stderr, "ERROR: Overlap check failed\n");
 res->check_errors++;
-/* Something is seriously wrong, so abort checking
- * this L2 table */
-goto fail;
+/*
+ * Something is seriously wrong, so abort checking
+ * this L2 table.
+ */
+return ret;
 }
 
 ret = bdrv_pwrite_sync(bs->file, l2e_offset,
@@ -1712,13 +1714,17 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 fprintf(stderr, "ERROR: Failed to overwrite L2 "
 "table entry: %s\n", strerror(-ret));
 res->check_errors++;
-/* Do not abort, continue checking the rest of this
- * L2 table's entries */
+/*
+ * Do not abort, continue checking the rest of this
+ * L2 table's entries.
+ */
 } else {
 res->corruptions--;
 res->corruptions_fixed++;
-/* Skip marking the cluster as used
- * (it is unused now) */
+/*
+ * Skip marking the cluster as used
+ * (it is unused now).
+ */
 continue;
 }
 }
@@ -1743,7 +1749,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
refcount_table_size,
offset, s->cluster_size);
 if (ret < 0) {
-goto fail;
+return ret;
 }
 }
 break;
@@ -1758,12 +1764,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 }
 }
 
-g_free(l2_table);
 return 0;
-
-fail:
-g_free(l2_table);
-return ret;
 }
 
 /*
-- 
2.29.2