From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43209) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS3za-0002L1-Ab for qemu-devel@nongnu.org; Thu, 11 Sep 2014 09:01:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XS3zR-0000mW-TJ for qemu-devel@nongnu.org; Thu, 11 Sep 2014 09:00:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10660) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS3zR-0000k1-Fo for qemu-devel@nongnu.org; Thu, 11 Sep 2014 09:00:45 -0400 Message-ID: <54119CF9.7000903@redhat.com> Date: Thu, 11 Sep 2014 07:00:41 -0600 From: Eric Blake MIME-Version: 1.0 References: <1410336832-22160-1-git-send-email-armbru@redhat.com> <1410336832-22160-9-git-send-email-armbru@redhat.com> <20140911113433.GC8522@irqsave.net> In-Reply-To: <20140911113433.GC8522@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gos2Nc95qJ9RxAhhLkGFTS1erO4T6mcIx" Subject: Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[] List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , Markus Armbruster Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --gos2Nc95qJ9RxAhhLkGFTS1erO4T6mcIx Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 09/11/2014 05:34 AM, Beno=C3=AEt Canet wrote: > The Wednesday 10 Sep 2014 =C3=A0 10:13:37 (+0200), Markus Armbruster wr= ote : >> device_name[] is can become non-empty only in bdrv_new_named() and >> bdrv_move_feature_fields(). The latter is used only to undo damage >> done by bdrv_swap(). The former is called only by blk_new_with_bs(). >> Therefore, when a BlockDriverState's device_name[] is non-empty, then >> it's owned by a BlockBackend. [lots of lines trimmed - it's not only okay, but desirable to trim out portions of a patch that you are okay with, in order to call attention to the problem spots that you are commenting on without making the reader have to scroll through pages of quoted context] >> =20 >> -const char *bdrv_get_device_name(BlockDriverState *bs) >> +const char *bdrv_get_device_name(const BlockDriverState *bs) >> { >> - return bs->device_name; >> + const char *name =3D bs->blk ? blk_name(bs->blk) : NULL; >> + return name ?: ""; >> } >=20 > Why not ? >=20 > return bs->blk ? blk_name(bs->blk) : ""; If I understand right, it was because blk_name(bs->blk) may return NULL, but this function is guaranteed to return non-NULL. >=20 > or=20 >=20 > if (bs->blk) { > return blk_name(bs->blk); > } >=20 > return ""; >=20 > Would it fail to do the job ? >=20 > Also we could have made sure that bdrv_get_device_name return either > a non empty string or NULL. >=20 > (We know blk_name will return a non empty string it's asserted) >=20 > This would need to change a few test all around the code but the final > code logic would be less convoluted: > -convert NULL to "" then test for not "" > would turn into > -return NULL test for not NULL Indeed, the logic of NULL vs. string is slightly easier than the logic of "" vs non-empty string; if we can guarantee that a non-NULL string will be non-empty. I'm not sure how much churn it would cause, though. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --gos2Nc95qJ9RxAhhLkGFTS1erO4T6mcIx 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 iQEcBAEBCAAGBQJUEZz5AAoJEKeha0olJ0NqJ7YIAJRjHtUl60MVK5JR9WR/YtBj Lw98L51BBsV9c60fxVe/0lds7q7Bgp9blxjTtH+L+w8uGD1ta/47BkV3cu0dKD9r xLI5PFIABVZAIJphA/+nPyXLfBLkVO6X7/rr0AFDBOJJTQLjeyBVsPb05XzIPDV8 ++i6fS0WuVav8eKUO1OU3REkxe/zwCsRMLTzDKJxIykoB4kDu2hVCru255A74AIg mmazSuzfzqe4YiG9X7rRAI870f4dAH7LopJFGn7fOrP/OsuVP8TzHSfOIHJFggk1 Vs4PI1okxXgNkkOGu5oTv+HNer0/MTGW4cG0oaJEjG8aFUPUCeY0boF8rgIdEF0= =Jik+ -----END PGP SIGNATURE----- --gos2Nc95qJ9RxAhhLkGFTS1erO4T6mcIx--