[libvirt] [PATCH] Sanitize qemu monitor output

[Daniel P. Berrange]

On Mon, Dec 01, 2008 at 05:36:34PM -0500, Cole Robinson wrote:
> The attached patch cleans up output we read from the qemu monitor. This
> is a simplified form of a patch I posted awhile ago. From the patch:
> 
> /* The monitor doesn't dump clean output after we have written to
>  * it. Every character we write dumps a bunch of useless stuff,
>  * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
>  * Try to throw away everything before the first full command
>  * occurence, and inbetween the command and the newline starting
>  * the response
>  */
> 
> This extra output makes our qemu log files _huge_ if doing things like
> polling disk/net stats, and prevents us from returning any useful error
> message printed from the monitor (say for media/disk eject, disk/net
> stats, etc).

Yes, this is quite horrific output thanks to QEMU's readline emulation
which we can't turn off.

> I've been running with this patch for a while and haven't hit any issues.
>          if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
> -            tmp[0] = '\0';
> +            /* Preserve the newline */
> +            tmp[1] = '\0';
> +
> +            /* The monitor doesn't dump clean output after we have written to
> +             * it. Every character we write dumps a bunch of useless stuff,
> +             * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
> +             * Try to throw away everything before the first full command
> +             * occurence, and inbetween the command and the newline starting
> +             * the response
> +             */
> +            if ((commptr = strstr(buf, cmd))) {
> +                char *dupptr = strchr(commptr, '\n');
> +                if (dupptr) {
> +                    char *tmpbuf = strdup(dupptr);
> +                    VIR_FREE(buf);
> +                    buf = strdup(tmpbuf);
> +                }
> +            }


This loooks a little overly complex to me, doesn't handle alloction failures
in strdup correctly & leaks tmpbuf. Can't we just simplify it to


     if ((commptr = strstr(buf, cmd)))
          memmove(buf, commptr, strlen(commptr)+1);

avoiding re-allocation entirely ?

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|