Libguestfs - Aug 2009 - [PATCH] Correct checks for dup failure in guestfs_launch

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
-------------- next part --------------
>From 8f1b06f64807239d4b4c923af4db8626a866ff6f Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at trick.home.annexia.org>
Date: Wed, 19 Aug 2009 09:37:44 +0100
Subject: [PATCH] guestfs_launch: Correct checks for dup failure.

---
 src/guestfs.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 58a0354..04bd4e8 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -1165,16 +1165,16 @@ guestfs_launch (guestfs_h *g)
     close (wfd[1]);
     close (rfd[0]);
 
-    int fail = 0;
-    fail |= dup (wfd[0]);
-    fail |= dup (rfd[1]);
-    close (wfd[0]);
-    close (rfd[1]);
-
-    if (fail) {
+    if (dup (wfd[0]) == -1) {
+    dup_failed:
       perror ("dup failed");
       _exit (1);
     }
+    if (dup (rfd[1]) == -1)
+      goto dup_failed;
+
+    close (wfd[0]);
+    close (rfd[1]);
 
 #if 0
     /* Set up a new process group, so we can signal this process
-- 
1.6.2.5

Richard W.M. Jones wrote:
> --
> Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
> New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
> programs, test, and build Windows installers. Over 70 libraries
supprt'd
> http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
>
>>From 8f1b06f64807239d4b4c923af4db8626a866ff6f Mon Sep 17 00:00:00 2001
> From: Richard Jones <rjones at trick.home.annexia.org>
> Date: Wed, 19 Aug 2009 09:37:44 +0100
> Subject: [PATCH] guestfs_launch: Correct checks for dup failure.
>
> ---
>  src/guestfs.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/src/guestfs.c b/src/guestfs.c
> index 58a0354..04bd4e8 100644
> --- a/src/guestfs.c
> +++ b/src/guestfs.c
> @@ -1165,16 +1165,16 @@ guestfs_launch (guestfs_h *g)
>      close (wfd[1]);
>      close (rfd[0]);
>
> -    int fail = 0;
> -    fail |= dup (wfd[0]);
> -    fail |= dup (rfd[1]);
> -    close (wfd[0]);
> -    close (rfd[1]);
> -
> -    if (fail) {
> +    if (dup (wfd[0]) == -1) {
> +    dup_failed:
>        perror ("dup failed");
>        _exit (1);
>      }
> +    if (dup (rfd[1]) == -1)
> +      goto dup_failed;
> +
> +    close (wfd[0]);
> +    close (rfd[1]);
>
>  #if 0
>      /* Set up a new process group, so we can signal this process
Good catch.
Here's an alternative patch to consider.
It has the marginal benefit of closing those two
file descriptors also when either of the dup syscalls fails.
However, that's no big deal here, since we're not checking for
close failure, and the very next statement is _exit.

The use of the |= operator might seem a little off-putting at
first, but when you get used to the idiom and see the general benefit
of fewer branches, and not having to "goto", maybe you'll prefer
it.

diff --git a/src/guestfs.c b/src/guestfs.c
index 0d54d65..53ef67d 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -1166,8 +1166,8 @@ guestfs_launch (guestfs_h *g)
     close (rfd[0]);

     int fail = 0;
-    fail |= dup (wfd[0]);
-    fail |= dup (rfd[1]);
+    fail |= (dup (wfd[0]) < 0);
+    fail |= (dup (rfd[1]) < 0);
     close (wfd[0]);
     close (rfd[1]);