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.

* 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.
This commit is contained in:
Eric Blake 2010-12-03 14:14:16 -07:00
parent ee11729d7f
commit cc5e2a849c
3 changed files with 108 additions and 15 deletions

View File

@ -365,9 +365,15 @@
</pre>
<p>
Once the command has finished executing, these buffers
will contain the output. It is the callers responsibility
to free these buffers.
Once the command has finished executing, these buffers will
contain the output. Allocation is guaranteed if virCommandRun
or virCommandWait succeed (if there was no output, then the
buffer will contain an allocated empty string); if the command
failed, then the buffers usually contain a best-effort
allocation of collected information (however, on an
out-of-memory condition, the buffer may still be NULL). The
caller is responsible for freeing registered buffers, since the
buffers are designed to persist beyond virCommandFree.
</p>
<h3><a name="directory">Setting working directory</a></h3>

View File

@ -533,11 +533,15 @@ virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf)
/*
* Capture the child's stdout to a string buffer
* Capture the child's stdout to a string buffer. *outbuf is
* guaranteed to be allocated after successful virCommandRun or
* virCommandWait, and is best-effort allocated after failed
* virCommandRun; caller is responsible for freeing *outbuf.
*/
void
virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
{
*outbuf = NULL;
if (!cmd || cmd->has_error)
return;
@ -553,11 +557,15 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
/*
* Capture the child's stderr to a string buffer
* Capture the child's stderr to a string buffer. *errbuf is
* guaranteed to be allocated after successful virCommandRun or
* virCommandWait, and is best-effort allocated after failed
* virCommandRun; caller is responsible for freeing *errbuf.
*/
void
virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf)
{
*errbuf = NULL;
if (!cmd || cmd->has_error)
return;
@ -747,6 +755,7 @@ virCommandProcessIO(virCommandPtr cmd)
int infd = -1, outfd = -1, errfd = -1;
size_t inlen = 0, outlen = 0, errlen = 0;
size_t inoff = 0;
int ret = 0;
/* With an input buffer, feed data to child
* via pipe */
@ -755,12 +764,27 @@ virCommandProcessIO(virCommandPtr cmd)
infd = cmd->inpipe;
}
/* With out/err buffer, the outfd/errfd
* have been filled with an FD for us */
if (cmd->outbuf)
/* With out/err buffer, the outfd/errfd have been filled with an
* FD for us. Guarantee an allocated string with partial results
* even if we encounter a later failure, as well as freeing any
* results accumulated over a prior run of the same command. */
if (cmd->outbuf) {
outfd = cmd->outfd;
if (cmd->errbuf)
if (VIR_REALLOC_N(*cmd->outbuf, 1) < 0) {
virReportOOMError();
ret = -1;
}
}
if (cmd->errbuf) {
errfd = cmd->errfd;
if (VIR_REALLOC_N(*cmd->errbuf, 1) < 0) {
virReportOOMError();
ret = -1;
}
}
if (ret == -1)
goto cleanup;
ret = -1;
for (;;) {
int i;
@ -791,7 +815,7 @@ virCommandProcessIO(virCommandPtr cmd)
continue;
virReportSystemError(errno, "%s",
_("unable to poll on child"));
return -1;
goto cleanup;
}
for (i = 0; i < nfds ; i++) {
@ -815,7 +839,7 @@ virCommandProcessIO(virCommandPtr cmd)
errno != EAGAIN) {
virReportSystemError(errno, "%s",
_("unable to write to child input"));
return -1;
goto cleanup;
}
} else if (done == 0) {
if (fds[i].fd == outfd)
@ -825,11 +849,10 @@ virCommandProcessIO(virCommandPtr cmd)
} else {
if (VIR_REALLOC_N(*buf, *len + done + 1) < 0) {
virReportOOMError();
return -1;
goto cleanup;
}
memcpy(*buf + *len, data, done);
*len += done;
(*buf)[*len] = '\0';
}
} else {
int done;
@ -841,7 +864,7 @@ virCommandProcessIO(virCommandPtr cmd)
errno != EAGAIN) {
virReportSystemError(errno, "%s",
_("unable to write to child input"));
return -1;
goto cleanup;
}
} else {
inoff += done;
@ -856,7 +879,13 @@ virCommandProcessIO(virCommandPtr cmd)
}
}
return 0;
ret = 0;
cleanup:
if (*cmd->outbuf)
(*cmd->outbuf)[outlen] = '\0';
if (*cmd->errbuf)
(*cmd->errbuf)[errlen] = '\0';
return ret;
}

View File

@ -610,6 +610,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 buffer is not an allocated empty string");
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 buffers are not allocated empty strings");
goto cleanup;
}
ret = 0;
cleanup:
virCommandFree(cmd);
VIR_FREE(outbuf);
VIR_FREE(errbuf);
return ret;
}
static int
mymain(int argc, char **argv)
{
@ -673,6 +730,7 @@ mymain(int argc, char **argv)
DO_TEST(test14);
DO_TEST(test15);
DO_TEST(test16);
DO_TEST(test17);
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}