2010/12/7 Eric Blake <eblake@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