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