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

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");

+        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");