From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWWBo-0001rb-9D for qemu-devel@nongnu.org; Mon, 31 Aug 2015 17:00:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWWBn-0006tP-6f for qemu-devel@nongnu.org; Mon, 31 Aug 2015 17:00:28 -0400 References: <1439939415-18180-1-git-send-email-mreitz@redhat.com> <1439939415-18180-4-git-send-email-mreitz@redhat.com> From: Eric Blake Message-ID: <55E4C062.4010008@redhat.com> Date: Mon, 31 Aug 2015 15:00:18 -0600 MIME-Version: 1.0 In-Reply-To: <1439939415-18180-4-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cBPg32E4njhBJoQuL6KMxM4Viwsd1vVdl" Subject: Re: [Qemu-devel] [PATCH v4 3/6] block: Add bdrv_filename() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --cBPg32E4njhBJoQuL6KMxM4Viwsd1vVdl Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08/18/2015 05:10 PM, Max Reitz wrote: > Split the part which actually refreshes the BlockDriverState.filename > field off of bdrv_refresh_filename() into a more generic function > bdrv_filename(), which first calls bdrv_refresh_filename() and then > stores a qemu-usable filename into the given buffer instead of > BlockDriverState.filename. >=20 > Since bdrv_refresh_filename() therefore no longer refreshes that field,= > some calls to that function have to be replaced by calls to > bdrv_filename() "manually" refreshing the BDS filename field (this is > only temporary). >=20 > Additionally, a wrapper function bdrv_filename_alloc() is added which > allocates a buffer of size PATH_MAX, call bdrv_filename() on that buffe= r > and returns it, since needing a temporary buffer for the filename is a > rather common pattern. >=20 > Signed-off-by: Max Reitz > --- > block.c | 39 ++++++++++++++++++++++++++++++++------- > block/blkverify.c | 3 ++- > block/quorum.c | 2 +- > include/block/block.h | 2 ++ > 4 files changed, 37 insertions(+), 9 deletions(-) >=20 > +/* First refreshes exact_filename and full_open_options by calling > + * bdrv_refresh_filename(). Then, if exact_filename is set, it is copi= ed into > + * the target buffer. Otherwise, full_open_options is converted to a J= SON > + * object, prefixed with "json:" (for use through the JSON pseudo prot= ocol) and > + * put there. > + * > + * If sz > 0, the string put into the buffer will always be null-termi= nated. > + * > + * Returns @dest. > + */ > +char *bdrv_filename(BlockDriverState *bs, char *dest, size_t sz) > +{ How does one tell if 'sz' was large enough, vs. too short and therefore the snprintf() truncated the resulting string? Would the code be any simpler if this always returned a freshly g_malloc'd string of the right length, rather than making the caller have to pre-allocate and guess whether the allocation was big enough? > + bdrv_refresh_filename(bs); > + > + if (sz > INT_MAX) { > + sz =3D INT_MAX; > + } > =20 > if (bs->exact_filename[0]) { > - pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename= ); > + pstrcpy(dest, sz, bs->exact_filename); > } else if (bs->full_open_options) { > QString *json =3D qobject_to_json(QOBJECT(bs->full_open_option= s)); > - snprintf(bs->filename, sizeof(bs->filename), "json:%s", > - qstring_get_str(json)); > + snprintf(dest, sz, "json:%s", qstring_get_str(json)); > QDECREF(json); > } > + > + return dest; In other words, I think it's very dangerous to use snprintf() without checking whether the result fit. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --cBPg32E4njhBJoQuL6KMxM4Viwsd1vVdl 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/ iQEcBAEBCAAGBQJV5MBiAAoJEKeha0olJ0NqwQEH/ihl27CWdxZAgiPcbvBtyhp0 gGaXhHt+56HBacHL32LBAuUJJwtptV+WRWLvWcBnr4z0TJqMSX+8OKgUB6UnT7oU TktEjJmmQ9h2Ffr0cscC+cvnGpwiypBYbdz+O/s6yFDUVuR2Tf6bYjlOIjcX8b4o f5AaFQoa1DAdQ3PdF6TZl94nW7KdlcEHgK2TuyvuLWjdReNc/Wle67SgScfwKjmx 7/LB9rKL4fwVFaxkqeXcCogkpaA//Y0BZVGVhI+NotlYtanHAL0MCX2pRq/C0CyG 3sQhn6v+R1eIpkTYxMlGLoio2Tv0i6drekC5l1tGHmvUj+accmOQ/tPHj106GCY= =YVpl -----END PGP SIGNATURE----- --cBPg32E4njhBJoQuL6KMxM4Viwsd1vVdl--