From e67ae619919acb654b96a6829d9161287f814e71 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 28 Jan 2011 14:22:39 -0700 Subject: [PATCH] build: avoid close, system * 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. --- 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 952bd6935f..701fafc1e7 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 f412a83035..ee04ca9ff8 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 > STDERR_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 38a78166b2..7157c51328 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 d411381fe9..59d099e491 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 *