qemu-devel
[Top][All Lists]
Advanced

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

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
Date: Fri, 28 Nov 2014 10:46:31 +0000
User-agent: Mutt/1.5.23 (2014-03-12)

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: pgprzuxTTmFlF.pgp
Description: PGP signature


reply via email to

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