Sometimes, its easier to run children with 2>&1 in shell notation, and just deal with stdout and stderr interleaved. This was already possible for fd handling; extend it to also work when doing string capture of a child process. * docs/internals/command.html.in: Document this. * src/util/command.c (virCommandSetErrorBuffer): Likewise. (virCommandRun, virExecWithHook): Implement it. * tests/commandtest.c (test14): Test it. * daemon/remote.c (remoteDispatchAuthPolkit): Use new command feature. --- In response to: https://www.redhat.com/archives/libvir-list/2012-January/msg01262.html Hmm, I thought I sent this earlier, but can't find it on the archives. daemon/remote.c | 11 +++-------- docs/internals/command.html.in | 5 ++++- src/util/command.c | 16 ++++++++++++++-- tests/commandtest.c | 26 +++++++++++++++++++++++++- 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 26ac4a6..1cea942 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2477,7 +2477,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, int status = -1; char *ident = NULL; bool authdismissed = 0; - char *pkout = NULL, *pkerr = NULL; + char *pkout = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); virCommandPtr cmd = NULL; @@ -2489,7 +2489,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, cmd = virCommandNewArgList(PKCHECK_PATH, "--action-id", action, NULL); virCommandSetOutputBuffer(cmd, &pkout); - virCommandSetErrorBuffer(cmd, &pkerr); + virCommandSetErrorBuffer(cmd, &pkout); VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { @@ -2548,17 +2548,12 @@ error: if (authdismissed) { virNetError(VIR_ERR_AUTH_CANCELLED, "%s", _("authentication cancelled by user")); - } else if (pkout || pkerr) { - virNetError(VIR_ERR_AUTH_FAILED, "%s %s", - pkerr ? pkerr : "", - pkout ? pkout : ""); } else { virNetError(VIR_ERR_AUTH_FAILED, "%s", - _("authentication failed")); + pkout && *pkout ? pkout : _("authentication failed")); } VIR_FREE(pkout); - VIR_FREE(pkerr); virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock); return -1; diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index 7dcf462..7bb9aa3 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -373,7 +373,10 @@ 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. + buffers are designed to persist beyond virCommandFree. It + is possible to pass the same pointer to both + virCommandSetOutputBuffer and virCommandSetErrorBuffer, in which + case the child process interleaves output into a single string. </p> <h3><a name="directory">Setting working directory</a></h3> diff --git a/src/util/command.c b/src/util/command.c index 6f51fbb..a2d5f84 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -464,7 +464,9 @@ virExecWithHook(const char *const*argv, } if (errfd != NULL) { - if (*errfd == -1) { + if (errfd == outfd) { + childerr = childout; + } else if (*errfd == -1) { if (pipe2(pipeerr, O_CLOEXEC) < 0) { virReportSystemError(errno, "%s", _("Failed to create pipe")); @@ -1428,7 +1430,10 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf) * guaranteed to be allocated after successful virCommandRun or * virCommandWait, and is best-effort allocated after failed * virCommandRun; caller is responsible for freeing *errbuf. - * This requires the use of virCommandRun. + * This requires the use of virCommandRun. It is possible to + * pass the same pointer as for virCommandSetOutputBuffer(), in + * which case the child process will interleave all output into + * a single string. */ void virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf) @@ -1940,6 +1945,13 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) cmd->inpipe = infd[1]; } + /* If caller requested the same string for stdout and stderr, then + * merge those into one string. */ + if (cmd->outbuf && cmd->outbuf == cmd->errbuf) { + cmd->errfdptr = &cmd->outfd; + cmd->errbuf = NULL; + } + /* If caller hasn't requested capture of stdout/err, then capture * it ourselves so we can log it. But the intermediate child for * a daemon has no expected output, and we don't want our diff --git a/tests/commandtest.c b/tests/commandtest.c index 9b9130c..3997d2c 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -508,6 +508,14 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) const char *errexpect = "BEGIN STDERR\n" "Hello World\n" "END STDERR\n"; + + char *jointactual = NULL; + const char *jointexpect = "BEGIN STDOUT\n" + "BEGIN STDERR\n" + "Hello World\n" + "Hello World\n" + "END STDOUT\n" + "END STDERR\n"; int ret = -1; virCommandSetInputBuffer(cmd, "Hello World\n"); @@ -523,7 +531,18 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) goto cleanup; virCommandFree(cmd); - cmd = NULL; + + cmd = virCommandNew(abs_builddir "/commandhelper"); + virCommandSetInputBuffer(cmd, "Hello World\n"); + virCommandSetOutputBuffer(cmd, &jointactual); + virCommandSetErrorBuffer(cmd, &jointactual); + if (virCommandRun(cmd, NULL) < 0) { + virErrorPtr err = virGetLastError(); + printf("Cannot run child %s\n", err->message); + goto cleanup; + } + if (!jointactual) + goto cleanup; if (!STREQ(outactual, outexpect)) { virtTestDifference(stderr, outexpect, outactual); @@ -533,6 +552,10 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virtTestDifference(stderr, errexpect, erractual); goto cleanup; } + if (!STREQ(jointactual, jointexpect)) { + virtTestDifference(stderr, jointexpect, jointactual); + goto cleanup; + } ret = checkoutput("test14"); @@ -540,6 +563,7 @@ cleanup: virCommandFree(cmd); VIR_FREE(outactual); VIR_FREE(erractual); + VIR_FREE(jointactual); return ret; } -- 1.7.7.6