[prev in list] [next in list] [prev in thread] [next in thread]
List: qemu-block
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v4 3/6] block: Add bdrv_filename()
From: Eric Blake <eblake () redhat ! com>
Date: 2015-08-31 21:00:18
Message-ID: 55E4C062.4010008 () redhat ! com
[Download RAW message or body]
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.
>
> 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).
>
> Additionally, a wrapper function bdrv_filename_alloc() is added which
> allocates a buffer of size PATH_MAX, call bdrv_filename() on that buffer
> and returns it, since needing a temporary buffer for the filename is a
> rather common pattern.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 39 ++++++++++++++++++++++++++++++++-------
> block/blkverify.c | 3 ++-
> block/quorum.c | 2 +-
> include/block/block.h | 2 ++
> 4 files changed, 37 insertions(+), 9 deletions(-)
>
> +/* First refreshes exact_filename and full_open_options by calling
> + * bdrv_refresh_filename(). Then, if exact_filename is set, it is copied into
> + * the target buffer. Otherwise, full_open_options is converted to a JSON
> + * object, prefixed with "json:" (for use through the JSON pseudo protocol) and
> + * put there.
> + *
> + * If sz > 0, the string put into the buffer will always be null-terminated.
> + *
> + * 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 = INT_MAX;
> + }
>
> 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 = qobject_to_json(QOBJECT(bs->full_open_options));
> - 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
["signature.asc" (application/pgp-signature)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic