From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42106) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XKXng-0001L4-Cu for qemu-devel@nongnu.org; Thu, 21 Aug 2014 15:13:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XKXna-0008Fv-Vw for qemu-devel@nongnu.org; Thu, 21 Aug 2014 15:13:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14566) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XKXna-0008EC-OF for qemu-devel@nongnu.org; Thu, 21 Aug 2014 15:13:26 -0400 Message-ID: <53F644D0.2020701@redhat.com> Date: Thu, 21 Aug 2014 13:13:20 -0600 From: Eric Blake MIME-Version: 1.0 References: <1408115786-13640-1-git-send-email-mreitz@redhat.com> <1408115786-13640-3-git-send-email-mreitz@redhat.com> <20140821185737.GB15636@nodalink.com> In-Reply-To: <20140821185737.GB15636@nodalink.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="djBKc1cU42tnF2cBWgfjKd6TKNMdAG7fh" Subject: Re: [Qemu-devel] [PATCH v2 2/9] qcow2: Factor out refcount accounting for check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , Max Reitz Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --djBKc1cU42tnF2cBWgfjKd6TKNMdAG7fh Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/21/2014 12:57 PM, Beno=C3=AEt Canet wrote: > On Fri, Aug 15, 2014 at 05:16:19PM +0200, Max Reitz wrote: >> Put the code for calculating the reference counts during qemu-img chec= k >> into an own function. >> >> Also, do not use g_realloc() for increasing the size of the in-memory >> refcount table, but rather g_try_realloc(). The "Also," may be a sign of doing two things in one patch that could have been two. >> >> Signed-off-by: Max Reitz >> --- >> block/qcow2-refcount.c | 188 ++++++++++++++++++++++++++++++----------= --------- >> 1 file changed, 115 insertions(+), 73 deletions(-) But overall, this patch seems like a reasonable refactor to me, splitting out a reusable piece. > Another point is that I think these two extractions patches will totaly= confuse > git blame the day we will need it. I disagree. When refactoring to split a large function into multiple smaller functions, where the original function calls out to a helper function for what it used to do inline, git blame tracks things perfectly. Someone following blame may end up on this commit as the source of a given line in its new location, but the blame viewer will also show this commit is a refactor, and it should be fairly easy to find where the line was moved from, and resume blaming even further. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --djBKc1cU42tnF2cBWgfjKd6TKNMdAG7fh Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJT9kTQAAoJEKeha0olJ0Nq1GQH/3UJiaWonhzGcSzO6JCL236l ytbxyZJdKye2zffT5Tg1AbLISQfK+ps0Yxy++/vhXslB5nZP0DNWpAv9Rt37nWtC LoYHJQ2fCd9Yes/Qkd1l8VPAlF4YYPWhya96fY9Y3G9dUrOU0xG4SOwxETVtIusG d5fbAci4FOCioE1VfucuDliMmdG1w/zkXCs0h4Rkb9oqD6TNlAy543dSALn+QY0z 87/Gli6H7RLOu6vYKWm+VeeeTesEuRB1zyt/RR/+k+7L8roZDu75Vw/qjjELJDeK wVtW/wCGDviyXNLAdrafMulIqvF0ZAKXQEUYwCWCq7ut2B3G42HRhMLKO1aRnkY= =YANn -----END PGP SIGNATURE----- --djBKc1cU42tnF2cBWgfjKd6TKNMdAG7fh--