From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41334) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjsAL-0007BD-PE for qemu-devel@nongnu.org; Wed, 07 Oct 2015 13:06:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZjsAH-0003QV-FN for qemu-devel@nongnu.org; Wed, 07 Oct 2015 13:06:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47951) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjsAH-0003QB-9U for qemu-devel@nongnu.org; Wed, 07 Oct 2015 13:06:05 -0400 References: <1441471439-6157-1-git-send-email-vsementsov@virtuozzo.com> <1441471439-6157-7-git-send-email-vsementsov@virtuozzo.com> <561452E5.1090005@redhat.com> From: Eric Blake Message-ID: <561550F5.304@redhat.com> Date: Wed, 7 Oct 2015 11:05:57 -0600 MIME-Version: 1.0 In-Reply-To: <561452E5.1090005@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="PWQTvr878hlpkJ4WehxOmwA5RPtq9ueW1" Subject: Re: [Qemu-devel] [PATCH 06/17] qcow2-dirty-bitmap: add qcow2_dirty_bitmap_load() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, stefanha@redhat.com, den@openvz.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --PWQTvr878hlpkJ4WehxOmwA5RPtq9ueW1 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/06/2015 05:01 PM, John Snow wrote: >> + ret =3D load_bitmap(bs_file, dirty_bitmap_table, bmh->dirty_bitma= p_table_size, bitmap); >> + if (ret < 0) { >> + error_setg_errno(errp, -ret, "Could not read bitmap from imag= e"); >> + goto finish; >> + } >> + >> +finish: >> + if (*errp !=3D NULL) { >> + bdrv_release_dirty_bitmap(bs_for, bitmap); >> + bitmap =3D NULL; >> + } >> + g_free(dirty_bitmap_table); >=20 >=20 > I think we're not supposed to be reaching into errp to check its > implementation detail like this ... the usual paradigm I see is just > "goto fail" or similar statements instead of checking for > error-or-success in a shared return block. >=20 If you have to make a decision based on whether an error was detected, then you MUST pass a local error (as your caller may have passed NULL because they don't care if you fail, even though you care if your helper fails). As in: Error *err =3D NULL; =2E.. if (ret < 0) { error_setg_errno(&err, -ret, "Could not read..."); } finish: if (err) { bdrv_release_dirty_bitmap(...); error_propagate(errp, err); bitmap =3D NULL; } In short, any code that does *errp is a potential NULL dereference. The comments in error.h help explain the paradigms. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --PWQTvr878hlpkJ4WehxOmwA5RPtq9ueW1 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWFVD1AAoJEKeha0olJ0NqhoEH/AjwMsxUS6eED/UoY6RLuQrS 4liLXnWJDjwjlrxHZkLf4lxVLze/btVEu5W1HIBgWeo0JOrU5XfFlNY099PMzev1 g1I15jxRgX6F2IXryKDnnsEhjm/tbCivrQEn14ZT83GAoTe8yGreaNO+zTgNKRR4 NIUdnYVLlwGaao5Z5033pkJJbTkLhkZLifM9H3MKeyMQ+JL8ReZPq+mWYsVPTM0d 6HANlreIL4kKoEiEGHifDRYgLAvK68TCePhT9aiec77o7P7sjjDL9l3RTFcl2W7e vqBcQRniO9/8/w9ghPNwacwmtiiFEakZFdddd4mYjYN8bzryDnAclo1JnJAnVhw= =/jRA -----END PGP SIGNATURE----- --PWQTvr878hlpkJ4WehxOmwA5RPtq9ueW1--