From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45128) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkZxq-0008K7-8Z for qemu-devel@nongnu.org; Fri, 09 Oct 2015 11:52:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkZxl-0007je-Ty for qemu-devel@nongnu.org; Fri, 09 Oct 2015 11:52:10 -0400 References: <1442907862-21376-1-git-send-email-wency@cn.fujitsu.com> <1442907862-21376-3-git-send-email-wency@cn.fujitsu.com> <561569A9.50609@redhat.com> From: Max Reitz Message-ID: <5617E29B.8000700@redhat.com> Date: Fri, 9 Oct 2015 17:51:55 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="sBvCPPRkUGbrEuE6W8kSs0bT0lqwPEQRU" Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v5 2/4] quorum: implement bdrv_add_child() and bdrv_del_child() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , Wen Congyang , qemu devel , Eric Blake , Markus Armbruster , Stefan Hajnoczi Cc: Kevin Wolf , zhanghailiang , qemu block , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , Gonglei , Yang Hongyang This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sBvCPPRkUGbrEuE6W8kSs0bT0lqwPEQRU Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 08.10.2015 10:12, Alberto Garcia wrote: > On Wed 07 Oct 2015 08:51:21 PM CEST, Max Reitz wrot= e: >>> + if (s->num_children =3D=3D s->max_children) { >>> + if (s->max_children >=3D INT_MAX) { >> >> Opposing Berto (:-)), I like >=3D even if the > part is actually >> impossible. I myself like to use constructs such as: >> >> for (i =3D 0; i < n; i++) { >> /* ... */ >> } >> if (i >=3D n) { >> /* ... */ >> } >> >> Even though @i can never exceed @n after the loop. This is because my >> way of thinking is "What if it could exceed @n? Then I'd like to take >> this branch as well." The same applies here. s->max_children can never= >> exceed INT_MAX, but if it could, we'd want that to be an error, too. >=20 > If s->max_children (and therefore s->num_children) could exceed the > upper limit then we would probably want to assert on that, because it > means that there's something seriously broken. The purpose of this code= > is to make sure that the upper limit is... well, an upper limit :-) If > that invariant no longer holds then I don't think we want a simple "Too= > many children" error. >=20 >>> + s->bs =3D g_renew(BlockDriverState *, s->bs, s->max_children= + 1); >>> + s->bs[s->num_children] =3D NULL; >>> + s->max_children++; >>> + } >> >> Just a suggestion, please feel free to ignore it completely: >> >> You can drop the s->max_children field and just always call g_renew() >> with s->num_children + 1 as the @count parameter. There shouldn't be >> any (visible) performance penalty, but it would simplify the code. >=20 > If s->num_children has decreased since the previous g_renew() call > (because the user called quorum_del_child()) that could actually reduce= > the array size. Yes, it could. And that would be just fine. ;-) We'd just keep the array exactly as big as it needs to be. I find that pretty intuitive. It's just counter-intuitive if you think one should never use realloc() for reducing the size of a buffer (and I know I myself tend to write my code thinking that). Max > Nothing would break though, at worst it would just be a= > bit counter-intuitive :-) >=20 > https://www.gnu.org/software/libc/manual/html_node/Changing-Block-Size.= html >=20 > Berto >=20 --sBvCPPRkUGbrEuE6W8kSs0bT0lqwPEQRU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWF+KbAAoJEDuxQgLoOKytYHUH/0/IcxzNLCs7Ml998gNSNNUs C7DdGpsBy1rEJWYpQg+M81ZB9oORrQdjlgitEZBuRiPiTvSngu4ejyQMzWdJgj9B W2/Zf7l4SezZdNwIOSywT489+lP79VF1zscirq+IWkRwHU8Luu4RYPNeOkv02rrL /bzRbhFHz53R84/U1jJef5h3lh60CQRx5dwYGeUu1is556VPEAENRFq5lDm4x3Pi 9uOX9np6qUz18UtgoI+aeBY1MwKWEFnNrCWyqsfyj+fKIyTLvWdgr7ph6ksczvvi EPzhjCn+pAE1wj2MJ0w3Rg6mh1iP5S9bFFC5hsR0eIp78+E7Lmb/3dRR1A+5hlw= =36b8 -----END PGP SIGNATURE----- --sBvCPPRkUGbrEuE6W8kSs0bT0lqwPEQRU--