From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50323) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XuJSI-00031o-PG for qemu-devel@nongnu.org; Fri, 28 Nov 2014 06:11:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XuJS9-0002Il-GO for qemu-devel@nongnu.org; Fri, 28 Nov 2014 06:11:18 -0500 Received: from mail-wg0-x231.google.com ([2a00:1450:400c:c00::231]:64966) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XuJS9-0002IX-An for qemu-devel@nongnu.org; Fri, 28 Nov 2014 06:11:09 -0500 Received: by mail-wg0-f49.google.com with SMTP id n12so315186wgh.8 for ; Fri, 28 Nov 2014 03:11:08 -0800 (PST) Date: Fri, 28 Nov 2014 10:46:31 +0000 From: Stefan Hajnoczi Message-ID: <20141128104631.GB13631@stefanha-thinkpad.redhat.com> References: <1416503198-17031-1-git-send-email-mreitz@redhat.com> <1416503198-17031-7-git-send-email-mreitz@redhat.com> <20141127150931.GF15586@stefanha-thinkpad.lan> <54773F10.3040102@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="LyciRD1jyfeSSjG0" Content-Disposition: inline In-Reply-To: <54773F10.3040102@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi --LyciRD1jyfeSSjG0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 an= d *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 =3D ROUND_UP(refcount_array_byte_size(s, *siz= e), > >>+ s->cluster_size); > >>+ size_t new_byte_size =3D ROUND_UP(refcount_array_byte_size(s, new_= size), > >>+ s->cluster_size); > >>+ uint16_t *new_ptr; > >>+ > >>+ if (new_byte_size <=3D old_byte_size) { > >>+ *size =3D new_size; > >>+ return 0; > >>+ } > >Why not realloc the array to the new smaller size? ... >=20 > Because such a call will actually never happen. I could replace this if () > by assert(new_byte_size >=3D old_byte_size); if (new_byte_size =3D=3D > 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 --LyciRD1jyfeSSjG0 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUeFKHAAoJEJykq7OBq3PIukkIAKAWFAb5E2ShUf/AeDFMIV1l unrk3jtmH2llBl+VPhxHa81/sJT8/VnwEcqIU4m2ErlymZFx1hHOM3YEfsQjIloR c+FVDtvjnhjfAvx5N/p18KozEHEGf50bItLD/7o1ASTkronT8iPMmqL/Nbx4eB/8 RUUJXstYlXm7DwwvShQP/vDEWr8Fa14SMM8NG5qTKkFMhvx7gNntuceDschv1FvD 9I62mNz1X5ug4j2h4rD8BQw7o23Prk3M/eL7ovZ6CZ2uHJFkB4gMSXIu5ykBTMjw BzW04EcnFXIBilUKElSraEiDL1AYN7EwOitbt6Du9bdikgmxWwxE4dbfo4dSAdE= =XG2W -----END PGP SIGNATURE----- --LyciRD1jyfeSSjG0--