6:02 p.m.
* src/fdstream.c (virFDStreamOpenFile, virFDStreamCreateFile):
Use VIR_FORCE_CLOSE instead of close.
* tests/commandtest.c (mymain): Likewise.
* tools/virsh.c (editFile): Use virCommand instead of system.
* src/util/util.c (__virExec): Special case preservation of std
file descriptors to child.
---
v2: make virCommand behave more like system. So I didn't do
any signal handling like system, but at least now you can
actually edit interactively. virCommandRun doesn't like
interactive sessions, so I have to use virCommandRunAsync
followed by virCommandWait. I also had to fix virExec.
src/fdstream.c | 6 ++--
src/util/util.c | 12 +++++----
tests/commandtest.c | 12 ++++++---
tools/virsh.c | 65 ++++++++++++++++++++++++++------------------------
4 files changed, 52 insertions(+), 43 deletions(-)
diff --git a/src/fdstream.c b/src/fdstream.c
index 952bd69..701fafc 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -1,7 +1,7 @@
/*
* fdstream.h: generic streams impl for file descriptors
*
- * Copyright (C) 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2009-2011 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -452,7 +452,7 @@ int virFDStreamOpenFile(virStreamPtr st,
return 0;
error:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
@@ -498,6 +498,6 @@ int virFDStreamCreateFile(virStreamPtr st,
return 0;
error:
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
diff --git a/src/util/util.c b/src/util/util.c
index f412a83..af14b2e 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -593,14 +593,16 @@ __virExec(const char *const*argv,
goto fork_error;
}
- VIR_FORCE_CLOSE(infd);
+ if (infd != STDIN_FILENO)
+ VIR_FORCE_CLOSE(infd);
VIR_FORCE_CLOSE(null);
- tmpfd = childout; /* preserve childout value */
- VIR_FORCE_CLOSE(tmpfd);
- if (childerr > 0 &&
+ if (childout != STDOUT_FILENO) {
+ tmpfd = childout; /* preserve childout value */
+ VIR_FORCE_CLOSE(tmpfd);
+ }
+ if (childerr > STDERR_FILENO &&
childerr != childout) {
VIR_FORCE_CLOSE(childerr);
- childout = -1;
}
/* Initialize full logging for a while */
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 38a7816..7157c51 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1,7 +1,7 @@
/*
* commandtest.c: Test the libCommand API
*
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -714,6 +714,7 @@ mymain(int argc, char **argv)
{
int ret = 0;
char cwd[PATH_MAX];
+ int fd;
abs_srcdir = getenv("abs_srcdir");
if (!abs_srcdir)
@@ -731,9 +732,12 @@ mymain(int argc, char **argv)
/* Kill off any inherited fds that might interfere with our
* testing. */
- close(3);
- close(4);
- close(5);
+ fd = 3;
+ VIR_FORCE_CLOSE(fd);
+ fd = 4;
+ VIR_FORCE_CLOSE(fd);
+ fd = 5;
+ VIR_FORCE_CLOSE(fd);
virInitialize();
diff --git a/tools/virsh.c b/tools/virsh.c
index d411381..59d099e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -56,6 +56,7 @@
#include "../daemon/event.h"
#include "configmake.h"
#include "threads.h"
+#include "command.h"
static char *progname;
@@ -9354,50 +9355,52 @@ static int
editFile (vshControl *ctl, const char *filename)
{
const char *editor;
- char *command;
- int command_ret;
+ virCommandPtr cmd;
+ int ret = -1;
+ int outfd = STDOUT_FILENO;
+ int errfd = STDERR_FILENO;
editor = getenv ("VISUAL");
- if (!editor) editor = getenv ("EDITOR");
- if (!editor) editor = "vi"; /* could be cruel & default to ed(1) here
*/
+ if (!editor)
+ editor = getenv ("EDITOR");
+ if (!editor)
+ editor = "vi"; /* could be cruel & default to ed(1) here */
/* Check that filename doesn't contain shell meta-characters, and
* if it does, refuse to run. Follow the Unix conventions for
* EDITOR: the user can intentionally specify command options, so
* we don't protect any shell metacharacters there. Lots more
* than virsh will misbehave if EDITOR has bogus contents (which
- * is why sudo scrubs it by default).
+ * is why sudo scrubs it by default). Conversely, if the editor
+ * is safe, we can run it directly rather than wasting a shell.
*/
- if (strspn (filename, ACCEPTED_CHARS) != strlen (filename)) {
- vshError(ctl,
- _("%s: temporary filename contains shell meta or other "
- "unacceptable characters (is $TMPDIR wrong?)"),
- filename);
- return -1;
+ if (strspn (editor, ACCEPTED_CHARS) != strlen (editor)) {
+ if (strspn (filename, ACCEPTED_CHARS) != strlen (filename)) {
+ vshError(ctl,
+ _("%s: temporary filename contains shell meta or other "
+ "unacceptable characters (is $TMPDIR wrong?)"),
+ filename);
+ return -1;
+ }
+ cmd = virCommandNewArgList("sh", "-c", NULL);
+ virCommandAddArgFormat(cmd, "%s %s", editor, filename);
+ } else {
+ cmd = virCommandNewArgList(editor, filename, NULL);
}
- if (virAsprintf(&command, "%s %s", editor, filename) == -1) {
- vshError(ctl,
- _("virAsprintf: could not create editing command: %s"),
- strerror(errno));
- return -1;
+ virCommandSetInputFD(cmd, STDIN_FILENO);
+ virCommandSetOutputFD(cmd, &outfd);
+ virCommandSetErrorFD(cmd, &errfd);
+ if (virCommandRunAsync(cmd, NULL) < 0 ||
+ virCommandWait(cmd, NULL) < 0) {
+ virshReportError(ctl);
+ goto cleanup;
}
+ ret = 0;
- command_ret = system (command);
- if (command_ret == -1) {
- vshError(ctl,
- _("%s: edit command failed: %s"), command, strerror(errno));
- VIR_FREE(command);
- return -1;
- }
- if (WEXITSTATUS(command_ret) != 0) {
- vshError(ctl,
- _("%s: command exited with non-zero status"), command);
- VIR_FREE(command);
- return -1;
- }
- VIR_FREE(command);
- return 0;
+cleanup:
+ virCommandFree(cmd);
+ return ret;
}
static char *
--
1.7.3.5