From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMrBi-0002JF-VA for qemu-devel@nongnu.org; Mon, 01 Apr 2013 22:43:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UMrBe-0007eW-VM for qemu-devel@nongnu.org; Mon, 01 Apr 2013 22:43:06 -0400 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:45875) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMrBe-0007cg-EU for qemu-devel@nongnu.org; Mon, 01 Apr 2013 22:43:02 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 2 Apr 2013 12:37:19 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 9FEF9357804E for ; Tue, 2 Apr 2013 13:42:43 +1100 (EST) Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r322St0565732748 for ; Tue, 2 Apr 2013 13:28:56 +1100 Received: from d23av02.au.ibm.com (loopback [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r322gBYG020771 for ; Tue, 2 Apr 2013 13:42:12 +1100 Message-ID: <515A4579.3060803@linux.vnet.ibm.com> Date: Tue, 02 Apr 2013 10:42:01 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1363961953-13561-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1363961953-13561-16-git-send-email-xiawenc@linux.vnet.ibm.com> <5159DD48.3030806@redhat.com> In-Reply-To: <5159DD48.3030806@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V10 15/17] block: dump to buffer for bdrv_image_info_dump() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com 于 2013-4-2 3:17, Eric Blake 写道: > 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. > I have considered it before, but here g_malloc0 always allocate a fixed half page size which brings less fragment than dynamic allocation according to string length, just as my comments in code: +/* Use buf instead of asprintf to reduce memory fragility. */ +char *bdrv_image_info_dump(char *buf, int buf_size, ImageInfo *info) Personally I'd like to avoid asprintf since there would be many times of reallocation for a single info dumping, not worthy I think, and there will not be much trouble if string truncate is tipped. -- Best Regards Wenchao Xia