From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XzO4r-0001DF-3r for qemu-devel@nongnu.org; Fri, 12 Dec 2014 06:08:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XzO4g-0003Gx-LQ for qemu-devel@nongnu.org; Fri, 12 Dec 2014 06:08:04 -0500 Received: from mail-wg0-x22d.google.com ([2a00:1450:400c:c00::22d]:51220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XzO4g-0003Gp-Dd for qemu-devel@nongnu.org; Fri, 12 Dec 2014 06:07:54 -0500 Received: by mail-wg0-f45.google.com with SMTP id b13so8774110wgh.32 for ; Fri, 12 Dec 2014 03:07:53 -0800 (PST) Date: Fri, 12 Dec 2014 11:07:51 +0000 From: Stefan Hajnoczi Message-ID: <20141212110751.GC22222@stefanha-thinkpad.redhat.com> References: <1417613866-25890-1-git-send-email-mreitz@redhat.com> <1417613866-25890-6-git-send-email-mreitz@redhat.com> <20141211105839.GE30812@stefanha-thinkpad.redhat.com> <54897A09.1070606@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="4jXrM3lyYWu4nBt5" Content-Disposition: inline In-Reply-To: <54897A09.1070606@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 05/26] qcow2: Use unsigned addend for update_refcount() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi --4jXrM3lyYWu4nBt5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 11, 2014 at 12:03:37PM +0100, Max Reitz wrote: > On 2014-12-11 at 11:58, Stefan Hajnoczi wrote: > >On Wed, Dec 03, 2014 at 02:37:25PM +0100, Max Reitz wrote: > >>@@ -530,8 +530,16 @@ found: > >> } > >> /* XXX: cache several refcount block clusters ? */ > >>+/* In order to decrease refcounts, set @addend to the two's complement= (giving a > >>+ * negative value and letting the implicit cast handle it is enough) a= nd set > >>+ * @decrease to true. @decrease must be false if the refcount should be > >>+ * increased. */ > >The first time I read this patch I missed this quirk and thought that a > >lot of places seemed to be doing the wrong thing with addend. > > > >This is likely to cause confusion, why not make uint16_t addend truly > >unsigned and leave the sign to bool decrease, as suggested by the > >function prototype? >=20 > Because it's very easy to call it with e.g. target_refcount - > current_refcount, and using an addition to apply the addend will always > work. >=20 > So, the code is a bit shorter by doing this. On the other hand, I don't h= ave > trouble making all callers do llabs(addend) or imaxabs(addend) (if the > absolute value is not known at compile time) and use addition or subtract= ion > in this function, depending on the boolean. I prefer the solution using the absolute value because it makes the code idiot-proof for me to read :). Thanks, Stefan --4jXrM3lyYWu4nBt5 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUisyHAAoJEJykq7OBq3PIAi4H/jMYG+QHfkgKM//DHSO/Vm7i vC6XyvesTYHfPDMc3ZwTjBS5nw3hZ//79x9IzTbjZa6njwwH4TPgr4VLbO8Ps4dl RrTisyw21mqA0BS5onGPWujN4E7lyvFxoJH67n6m1/6TEn/6XQDOvCSkxFl1sBey DWG01uoAotyQahJQ38oAST7cIPCh57DkZ3X5SbZwcyes6t9f3j21M5/tYzRTHDPf QxISMCTCRDrSDverW69vFlQ74OHKMLAUN0TwsPS0LBvczVIPX23yh/lI0fwLkjHh vSToRGBtqKbs4oDEXU1LL9PiW2v/6pq6CnqXn9WWLCqb/yIM28zzxHlE+extbxs= =+qse -----END PGP SIGNATURE----- --4jXrM3lyYWu4nBt5--