From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55939) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkwal-00086O-JQ for qemu-devel@nongnu.org; Thu, 15 May 2014 10:25:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wkwac-0004Yf-B9 for qemu-devel@nongnu.org; Thu, 15 May 2014 10:25:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35564) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkwac-0004Yb-2A for qemu-devel@nongnu.org; Thu, 15 May 2014 10:24:54 -0400 Date: Thu, 15 May 2014 16:24:48 +0200 From: Kevin Wolf Message-ID: <20140515142448.GI3822@noname.redhat.com> References: <5374CC3B.8090007@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="4Ckj6UjgE2iN1+kY" Content-Disposition: inline In-Reply-To: <5374CC3B.8090007@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/5] block: add helper function to determine if a BDS is in a chain List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: benoit.canet@irqsave.net, pkrempa@redhat.com, famz@redhat.com, Jeff Cody , qemu-devel@nongnu.org, stefanha@redhat.com --4Ckj6UjgE2iN1+kY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 15.05.2014 um 16:16 hat Eric Blake geschrieben: > On 05/14/2014 09:20 PM, Jeff Cody wrote: > > This is a small helper function, to determine if 'base' is in the > > chain of BlockDriverState 'top'. It returns true if it is in the chain, > > and false otherwise. > >=20 > > If either argument is NULL, it will also return false. > >=20 > > Signed-off-by: Jeff Cody > > --- > > block.c | 9 +++++++++ > > include/block/block.h | 1 + > > 2 files changed, 10 insertions(+) > >=20 >=20 > > =20 > > +bool bdrv_is_in_chain(BlockDriverState *top, BlockDriverState *base) >=20 > No doc comments inline, and not everyone has the commit message handy. > Which means someone trying to learn what this command does has to read > the function. Maybe copy the commit message into code comments as well. >=20 > Bikeshedding: If I'm reading code, and see bdrv_is_in_chain(a, b), my > first inclination would be to read that as "return true if node a is in > chain b". But if I were to see bdrv_chain_contains(a, b), I would parse > that as "return true if chain a contains node b". I think you either > want to swap argument order, or rename the function. I haven't read the patch yet, so I can't say much about the context, but just from seeing the function names I think I agree with you. Kevin --4Ckj6UjgE2iN1+kY Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTdM4wAAoJEH8JsnLIjy/WdaEQAIlNhhn2UMcqqLrc2KjrLxW5 CUm3Wjr6B4am94dZjFX8ZjXZI+ceTx3Kqrhsi90gGzE/sAhHY+B+chVn4+cdta+h wMiff5YrGWqWHhf2uLWrji8kJthEnq1NyKuchC6TcggHC/0ySueKjEmRqchJ8y4Z zHPs5pZyQl+kdCHaYwZEd//QZzjhFzUwfJkWeJqRTkSWk8f8XNTKMMNWNH6cnPgm Xfi2dzUhPvsDLrhvyolfma+3XfYBrClAG3u2zkdXRGFYFu628+sbANTahb/01Li8 wfwJ2PNExvagA4otY4BzJ2mdM9Nt2gL7dMb1fYyq5Zuqn3rHYtExEWNPKOFv+wDB Ob3m3s6idcmx+xvYvC8MvxJW3m7HFXl0W5NR9ReK/1c8+N3FvLSKeN2V4mXwewXp D3XBSAAq4ZoYdVhCaKR4qUz+CGrVi0rff/foz4Ts4kgIM9E03ROK/QacqKYBNf/k RJF4sJRkDSJYywFwuAGbozDDesWIrGBcMMs1Mq0joh5VYSwkKyr4IFuJI19JodUU pL9hzpr4mZz5p+JT8YfGBhgDxDnHG87TWJ7oKqDOhvCJjMxuFqeS17j/z8vVzPnI QrpUxelQif7s+HNtsTqb6durs1IFDrtUsYJ4qi1a84SFfc+VzVMS68W+VZKRGRTD 11ACYj4i7m65uoItpcbB =yNAy -----END PGP SIGNATURE----- --4Ckj6UjgE2iN1+kY--