qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 07/21] qcow2: Helper for refcount array size cal


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH 07/21] qcow2: Helper for refcount array size calculation
Date: Tue, 11 Nov 2014 09:37:16 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0

On 2014-11-10 at 23:49, Eric Blake wrote:
On 11/10/2014 06:45 AM, Max Reitz wrote:
Add a helper function which correctly calculates the byte size of a
refcount array for any refcount order, and use that function.

Signed-off-by: Max Reitz <address@hidden>
---
  block/qcow2-refcount.c | 39 ++++++++++++++++++++++++++++-----------
  1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 16652da..cfb4807 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1132,6 +1132,20 @@ fail:
  /* refcount checking functions */
+static size_t refcount_array_byte_size(BDRVQcowState *s, uint64_t entries)
+{
+    if (s->refcount_order < 3) {
+        /* sub-byte width */
+        int shift = 3 - s->refcount_order;
+        return (entries + (1 << shift) - 1) >> shift;
+    } else if (s->refcount_order == 3) {
+        /* byte width */
+        return entries;
+    } else {
+        /* multiple bytes wide */
+        return entries << (s->refcount_order - 3);
+    }
A comment proving why this can't overflow might be nice (if I analyzed
correctly, entries will be computed by file size / clusters, and in the
worst case, the smallest cluster and largest refcount_order results in
'(size >> 9) << (6 - 3)' which is still safe).

Yes, will do.

@@ -1161,12 +1175,13 @@ static int inc_refcounts(BlockDriverState *bs,
          k = cluster_offset >> s->cluster_bits;
          if (k >= *refcount_table_size) {
              int64_t old_refcount_table_size = *refcount_table_size;
+            size_t new_byte_size;
              void *new_refcount_table;
*refcount_table_size = k + 1;
-            new_refcount_table = g_try_realloc(*refcount_table,
-                                               *refcount_table_size *
-                                               s->refcount_bits / 8);
+            new_byte_size = refcount_array_byte_size(s, *refcount_table_size);
+
+            new_refcount_table = g_try_realloc(*refcount_table, new_byte_size);
Yay - this addresses one of my possible overflow comments on 6/21.

I wonder if the series would have less churn if you rearranged this
patch to come before 6/21.

Why not, I'll add an __attribute__((used)) to it (which should be fine for the duration of a single patch).

Max



reply via email to

[Prev in Thread] Current Thread [Next in Thread]