Re: [Qemu-devel] [PATCH 2/2] Monitor: Make output buffer dynamic

[Paolo Bonzini <pbonzini@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 <lcapitulino@redhat.com>
> ---
>  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;
>