Re: [libvirt] [PATCHv2 1/2] build: avoid close, system
2011/1/29 Eric Blake <eblake(a)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/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/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);
Took me a moment to understand this. I think in case you pass parent's
std{in,out,err} to child's std{in,out,err} this works. But when you
exchange stdout and stderr like this: parent std{in,err,out} -> child
std{in,out,err}, then childout != STDOUT_FILENO is wrong and should be
childout > STDERR_FILENO, otherwise you could close the parent's
stderr here.
+ }
+ if (childerr > STDERR_FILENO &&
childerr != childout) {
VIR_FORCE_CLOSE(childerr);
- childout = -1;
Technically it's okay to remove this like as childout isn't accessed
afterwards anymore. But by using the tmpfd variable we tricked
VIR_FORCE_CLOSE and should finish it's job here.
ACK, with that comments addressed.
Mathias