[libvirt] [PATCHv3] command: improve behavior on no output

[Matthias Bolte]

2010/12/7 Eric Blake <eblake at redhat.com>:
> Guarantee that outbuf/errbuf are allocated on success, even if to the
> empty string.  Caller always has to free the result, and empty output
> check requires checking if *outbuf=='\0'.  Makes the API easier to use
> safely.  Failure is best effort allocation (some paths, like
> out-of-memory, cannot allocate a buffer, but most do), so caller must
> free buffer on failure; but this now guarantees that VIR_FREE(buf)
> will be safe.
>
> * docs/internals/command.html.in: Update documentation.
> * src/util/command.c (virCommandSetOutputBuffer)
> (virCommandSetErrorBuffer, virCommandProcessIO) Guarantee empty
> string on no output.
> * tests/commandtest.c (test17): New test.
> ---
>
> Everyone liked the safety of v2 over the speed of v1; and in looking
> at it more, I found even more ways to make it safer.  Hence this v3.
>
>  docs/internals/command.html.in |   12 ++++++--
>  src/util/command.c             |   53 ++++++++++++++++++++++++++++--------
>  tests/commandtest.c            |   58 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+), 15 deletions(-)

> diff --git a/tests/commandtest.c b/tests/commandtest.c
> index b7261e9..20e56bc 100644
> --- a/tests/commandtest.c
> +++ b/tests/commandtest.c
> @@ -595,6 +595,63 @@ cleanup:
>     return ret;
>  }
>
> +/*
> + * Test string handling when no output is present.
> + */
> +static int test17(const void *unused ATTRIBUTE_UNUSED)
> +{
> +    virCommandPtr cmd = virCommandNew("/bin/true");
> +    int ret = -1;
> +    char *outbuf;
> +    char *errbuf;
> +
> +    virCommandSetOutputBuffer(cmd, &outbuf);
> +    if (outbuf != NULL) {
> +        puts("buffer not sanitized at registration");
> +        goto cleanup;
> +    }
> +
> +    if (virCommandRun(cmd, NULL) < 0) {
> +        virErrorPtr err = virGetLastError();
> +        printf("Cannot run child %s\n", err->message);
> +        goto cleanup;
> +    }
> +
> +    if (!outbuf || *outbuf) {
> +        puts("output string not allocated");

The error message doesn't fit to the *outbuf != \0 test.

> +        goto cleanup;
> +    }
> +    VIR_FREE(outbuf);
> +    if ((outbuf = strdup("should not be leaked")) == NULL) {
> +        puts("test framework failure");
> +        goto cleanup;
> +    }
> +
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +    if (errbuf != NULL) {
> +        puts("buffer not sanitized at registration");
> +        goto cleanup;
> +    }
> +
> +    if (virCommandRun(cmd, NULL) < 0) {
> +        virErrorPtr err = virGetLastError();
> +        printf("Cannot run child %s\n", err->message);
> +        goto cleanup;
> +    }
> +
> +    if (!outbuf || *outbuf || !errbuf || *errbuf) {
> +        puts("output strings not allocated");

The error message doesn't fit to the *{out|err}buf != \0 test.

Also what about the case when running /bin/true prints to {std|err}out
for some reason? :)

ACK.

Matthias