[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V10 15/17] block: dump to buffer for bdrv_image_
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH V10 15/17] block: dump to buffer for bdrv_image_info_dump() |
Date: |
Mon, 01 Apr 2013 13:17:28 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 |
On 03/22/2013 08:19 AM, Wenchao Xia wrote:
> This allow hmp use this function, just like qemu-img.
> It also returns a pointer now to make it easy to use.
>
>
> -void bdrv_image_info_dump(ImageInfo *info)
> +GCC_FMT_ATTR(3, 4)
> +static void snprintf_tail(char **p_buf, int *p_size, const char *fmt, ...)
Yuck. I'm too worried that you are likely to cause truncation when you
exceed the bounds of the fixed-width buffer. And you can't argue that
this is here to avoid malloc pressure, since...
>
> +#define IMAGE_INFO_BUF_SIZE (2048)
> static void dump_human_image_info_list(ImageInfoList *list)
> {
> ImageInfoList *elem;
> bool delim = false;
> + char *buf = g_malloc0(IMAGE_INFO_BUF_SIZE);
...you are doing a malloc for the original buffer in the first place.
I'd much rather see use of g_string_append_printf or some similar glib
interface that manages a dynamically-sized output buffer to begin with,
than to attempt to force the output to fit in a fixed-width malloc'd buffer.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH V10 15/17] block: dump to buffer for bdrv_image_info_dump(),
Eric Blake <=