From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50245) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b87UP-0005oO-Cu for qemu-devel@nongnu.org; Wed, 01 Jun 2016 10:51:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b87UM-0007B2-56 for qemu-devel@nongnu.org; Wed, 01 Jun 2016 10:51:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58120) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b87UL-0007Al-Sh for qemu-devel@nongnu.org; Wed, 01 Jun 2016 10:51:18 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1836325897 for ; Wed, 1 Jun 2016 14:51:17 +0000 (UTC) References: <1464712890-14262-1-git-send-email-eblake@redhat.com> <1464712890-14262-4-git-send-email-eblake@redhat.com> <87y46p4afp.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <574EF664.5060205@redhat.com> Date: Wed, 1 Jun 2016 08:51:16 -0600 MIME-Version: 1.0 In-Reply-To: <87y46p4afp.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="OW8KPMplcfMap4gBu1VQMhH6rJP9EX0Jh" Subject: Re: [Qemu-devel] [PATCH v2 3/3] qapi: Fix memleak in string visitors on int lists List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Paolo Bonzini , "Michael S. Tsirkin" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --OW8KPMplcfMap4gBu1VQMhH6rJP9EX0Jh Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/01/2016 01:47 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Commit 7f8f9ef1 introduced the ability to store a list of >> integers as a sorted list of ranges, but when merging ranges, >> it leaks one or more ranges. It was also using range_get_last() >> incorrectly within range_compare() (a range is a start/end pair, >> but range_get_last() is for start/len pairs), and will also >> mishandle a range ending in UINT64_MAX (remember, we document >> that no range covers 2**64 bytes, but that ranges that end on >> UINT64_MAX have end < begin). >> >> >> - if (!list) { >> - list =3D g_list_insert_sorted(list, data, range_compare); >> - return list; >> + /* Range lists require no empty ranges */ >> + assert(data->begin < data->end || (data->begin && !data->end)); >=20 > Consider { begin =3D 0, end =3D 0 }. >=20 > Since zero @end means 2^64, this encodes the (non-empty) range > 0..2^64-1. Or else it means an uninitialized range. My argument is that no range can contain 2^64 bytes, and therefore the only possible range that would be that large (0..2^64-1) is unrepresentable, therefore, if end =3D=3D 0,= begin must be non-zero for the range to be valid as an initialized range.= >=20 > range.h's comment >=20 > * Notes: > * - ranges must not wrap around 0, but can include the last byte ~0x= 0LL. > * - this can not represent a full 0 to ~0x0LL range. >=20 > appears to be wrong. The actual limitation is "can't represent ranges > wrapping around zero, and can't represent the empty range starting at > zero." Would you like to correct it? I'm not sure what corrections it needs, though. >=20 > I'm afraid range.h is too clever by half. Unfortunately true. >=20 >> + >> + for (l =3D list; l && range_compare(l->data, data) < 0; l =3D l->= next) { >> + /* Skip all list elements strictly less than data */ >> } >=20 > Let's put the comment before the loop. It describes the whole loop. > Also makes the emptiness of the body more obvious. Sure. >=20 > I think I could fix up things on commit (assuming we agree on what need= s > fixing). >=20 Adding other authors of range.h for their opinions... --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --OW8KPMplcfMap4gBu1VQMhH6rJP9EX0Jh 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/ iQEcBAEBCAAGBQJXTvZkAAoJEKeha0olJ0NqUfQH/3JO/jLGfyKwBaafEPiYcuiO R809UTtDnubftf12ti//5JUFe+Qhdn9xZ1XaSnhIV3skutca7T3CZf0GC+bl/WQT dT11vyjtVp+++3DEL+PsRZKZRHPVVx46yUEuCfzA9GbKFshPAPYamx97rzm4n6v1 vgN5VEj6g1QSTz5NO44HURJB9IZmm7kYoQ34rtEH7WeasfZWuxb9E8SGfTtsanYH Aik1dRxd28CouHjKthvaEJ/oml5Z82SjwY0FRP/aRjjkaaDaEsjyQk6WTEBNHWah zeJ+Mtxr02Pt3eTGVqvJhqKy3XTetU+alF+2iqX+1YKUCKaVEQUrNwpJR2UJFkk= =VmKI -----END PGP SIGNATURE----- --OW8KPMplcfMap4gBu1VQMhH6rJP9EX0Jh--