From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKEcJ-0003x3-GC for qemu-devel@nongnu.org; Mon, 25 Mar 2013 17:07:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UKEcI-0005ms-5j for qemu-devel@nongnu.org; Mon, 25 Mar 2013 17:07:43 -0400 Received: from mail-we0-x22f.google.com ([2a00:1450:400c:c03::22f]:42494) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKEcH-0005md-SM for qemu-devel@nongnu.org; Mon, 25 Mar 2013 17:07:42 -0400 Received: by mail-we0-f175.google.com with SMTP id t11so3422897wey.6 for ; Mon, 25 Mar 2013 14:07:41 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5150BC98.5060906@redhat.com> Date: Mon, 25 Mar 2013 22:07:36 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1364240439-23450-1-git-send-email-lcapitulino@redhat.com> <1364240439-23450-3-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1364240439-23450-3-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] Monitor: Make output buffer dynamic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: fred.konrad@greensocs.com, qemu-devel@nongnu.org, kraxel@redhat.com Il 25/03/2013 20:40, Luiz Capitulino ha scritto: > Commit f628926bb423fa8a7e0b114511400ea9df38b76a changed monitor_flush() > to retry on qemu_chr_fe_write() errors. However, the Monitor's output > buffer can keep growing while the retry is not issued and this can > cause the buffer to overflow. > > To reproduce this issue, just start qemu and type on the Monitor: > > (qemu) ? > > This will cause the assertion to trig. > > To fix this problem this commit makes the Monitor buffer dynamic, > which means that it can grow as much as needed. What about using a GString instead? Paolo > Signed-off-by: Luiz Capitulino > --- > monitor.c | 42 +++++++++++++++++++++++++----------------- > 1 file changed, 25 insertions(+), 17 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 2d9e887..c93dfe7 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -188,8 +188,7 @@ struct Monitor { > int reset_seen; > int flags; > int suspend_cnt; > - uint8_t outbuf[1024]; > - int outbuf_index; > + QString *outbuf; > ReadLineState *rs; > MonitorControl *mc; > CPUArchState *mon_cpu; > @@ -271,45 +270,52 @@ static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond, > void monitor_flush(Monitor *mon) > { > int rc; > + size_t len; > + const char *buf; > + > + buf = qstring_get_str(mon->outbuf); > + len = qstring_get_length(mon->outbuf); > > - if (mon && mon->outbuf_index != 0 && !mon->mux_out) { > - rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index); > - if (rc == mon->outbuf_index) { > + if (mon && len && !mon->mux_out) { > + rc = qemu_chr_fe_write(mon->chr, (const uint8_t *) buf, len); > + if (rc == len) { > /* all flushed */ > - mon->outbuf_index = 0; > + QDECREF(mon->outbuf); > + mon->outbuf = qstring_new(); > return; > } > if (rc > 0) { > /* partinal write */ > - memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc); > - mon->outbuf_index -= rc; > + QString *tmp = qstring_from_str(buf + rc); > + QDECREF(mon->outbuf); > + mon->outbuf = tmp; > } > qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon); > } > } > > -/* flush at every end of line or if the buffer is full */ > +/* flush at every end of line */ > static void monitor_puts(Monitor *mon, const char *str) > { > char c; > > for(;;) { > - assert(mon->outbuf_index < sizeof(mon->outbuf) - 1); > c = *str++; > if (c == '\0') > break; > - if (c == '\n') > - mon->outbuf[mon->outbuf_index++] = '\r'; > - mon->outbuf[mon->outbuf_index++] = c; > - if (mon->outbuf_index >= (sizeof(mon->outbuf) - 1) > - || c == '\n') > + if (c == '\n') { > + qstring_append_chr(mon->outbuf, '\r'); > + } > + qstring_append_chr(mon->outbuf, c); > + if (c == '\n') { > monitor_flush(mon); > + } > } > } > > void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) > { > - char buf[4096]; > + char *buf; > > if (!mon) > return; > @@ -318,8 +324,9 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) > return; > } > > - vsnprintf(buf, sizeof(buf), fmt, ap); > + buf = g_strdup_vprintf(fmt, ap); > monitor_puts(mon, buf); > + g_free(buf); > } > > void monitor_printf(Monitor *mon, const char *fmt, ...) > @@ -4732,6 +4739,7 @@ void monitor_init(CharDriverState *chr, int flags) > } > > mon = g_malloc0(sizeof(*mon)); > + mon->outbuf = qstring_new(); > > mon->chr = chr; > mon->flags = flags; >