2011/1/28 Eric Blake <eblake@redhat.com>:
* 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/fdstream.c | 6 ++-- tests/commandtest.c | 12 +++++++--- tools/virsh.c | 59 +++++++++++++++++++++++--------------------------- 3 files changed, 38 insertions(+), 39 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index cd54174..cf482e3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c
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 */
When VISUAL and EDITOR isn't set we fallback to vi here, but ...
/* 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); }
... here you made it fallback to editor instead. Shouldn't this be consistent and fallback to the same in both cases? Anyway, that's minor and doesn't affect my ACK. Matthias