Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation

[Stefan Hajnoczi <stefanha@gmail.com>]

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

On Thu, Nov 27, 2014 at 04:11:12PM +0100, Max Reitz wrote:
> On 2014-11-27 at 16:09, Stefan Hajnoczi wrote:
> >On Thu, Nov 20, 2014 at 06:06:22PM +0100, Max Reitz wrote:
> >>+/**
> >>+ * Reallocates *array so that it can hold new_size entries. *size must contain
> >>+ * the current number of entries in *array. If the reallocation fails, *array
> >>+ * and *size will not be modified and -errno will be returned. If the
> >>+ * reallocation is successful, *array will be set to the new buffer and *size
> >>+ * will be set to new_size. The size of the reallocated refcount array buffer
> >>+ * will be aligned to a cluster boundary, and the newly allocated area will be
> >>+ * zeroed.
> >>+ */
> >>+static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
> >>+                                  int64_t *size, int64_t new_size)
> >>+{
> >>+    /* Round to clusters so the array can be directly written to disk */
> >>+    size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
> >>+                                    s->cluster_size);
> >>+    size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
> >>+                                    s->cluster_size);
> >>+    uint16_t *new_ptr;
> >>+
> >>+    if (new_byte_size <= old_byte_size) {
> >>+        *size = new_size;
> >>+        return 0;
> >>+    }
> >Why not realloc the array to the new smaller size? ...
> 
> Because such a call will actually never happen. I could replace this if ()
> by assert(new_byte_size >= old_byte_size); if (new_byte_size ==
> old_byte_size), but as I said before, I'm not a friend of assertions when
> the code can deal perfectly well with the "unsupported" case.

It is simpler to put an if statement around the memset.  Then the
function actually frees unused memory and readers don't wonder why you
decided not to shrink the array.

Less code and slightly better behavior.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]