2010/12/7 Eric Blake <eblake@xxxxxxxxxx>:
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list