thr3ads.net - Libguestfs - [Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls [Aug 2009]

If this information is useful, please help other people find it:
Share via:

Jim Meyering

2009-Aug-19 08:34 UTC

[Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls

There were a few unchecked syscalls in fish.c
>From ba8b8b0684a03b6e6fbb939ed7e1cbf5e1000092 Mon Sep 17 00:00:00 2001From: Jim Meyering <meyering at redhat.com>
Date: Wed, 19 Aug 2009 10:01:07 +0200
Subject: [PATCH libguestfs] guestfish: detect a few more failed syscalls

* fish/fish.c (issue_command): Detect/diagnose more failed syscalls.
---
 fish/fish.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/fish/fish.c b/fish/fish.c
index 830617b..e6cd270 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -750,8 +750,14 @@ issue_command (const char *cmd, char *argv[], const char
*pipecmd)
   if (pipecmd) {
     int fd[2];

-    fflush (stdout);
-    pipe (fd);
+    if (fflush (stdout)); {
+      perror ("failed to flush standard output");
+      return -1;
+    }
+    if (pipe (fd)) {
+      perror ("pipe failed");
+      return -1;
+    }
     pid = fork ();
     if (pid == -1) {
       perror ("fork");
@@ -760,7 +766,10 @@ issue_command (const char *cmd, char *argv[], const char
*pipecmd)

     if (pid == 0) {		/* Child process. */
       close (fd[1]);
-      dup2 (fd[0], 0);
+      if (dup2 (fd[0], 0) < 0) {
+        perror ("dup2 of stdin failed");
+        _exit (1);
+      }

       r = system (pipecmd);
       if (r == -1) {
@@ -770,9 +779,16 @@ issue_command (const char *cmd, char *argv[], const char
*pipecmd)
       _exit (WEXITSTATUS (r));
     }

-    stdout_saved_fd = dup (1);
+    if ((stdout_saved_fd = dup (1)) < 0) {
+      perror ("failed to dup stdout");
+      return -1;
+    }
     close (fd[0]);
-    dup2 (fd[1], 1);
+    if (dup2 (fd[1], 1)) {
+      perror ("failed to dup stdout");
+      close (stdout_saved_fd);
+      return -1;
+    }
     close (fd[1]);
   }

--
1.6.4.378.g88f2f

Jim Meyering

2009-Aug-19 08:59 UTC

head link

[Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls

Jim Meyering wrote:> There were a few unchecked syscalls in fish.c
>
>>From ba8b8b0684a03b6e6fbb939ed7e1cbf5e1000092 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Wed, 19 Aug 2009 10:01:07 +0200
> Subject: [PATCH libguestfs] guestfish: detect a few more failed syscalls
>
> * fish/fish.c (issue_command): Detect/diagnose more failed syscalls.
...> -    dup2 (fd[1], 1);
> +    if (dup2 (fd[1], 1)) {
Rich noticed that this test is wrong (should test < 0).  Thanks!
Here's a fixed patch (also made the pipe test for < 0, though there
it's not important):
>From afee87d3b908bd1b6970e5b141452841d8dd5da8 Mon Sep 17 00:00:00 2001From: Jim Meyering <meyering at redhat.com>
Date: Wed, 19 Aug 2009 10:01:07 +0200
Subject: [PATCH libguestfs] guestfish: detect a few more failed syscalls

* fish/fish.c (issue_command): Detect/diagnose more failed syscalls.
---
 fish/fish.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/fish/fish.c b/fish/fish.c
index 830617b..5b2cf45 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -750,8 +750,14 @@ issue_command (const char *cmd, char *argv[], const char
*pipecmd)
   if (pipecmd) {
     int fd[2];

-    fflush (stdout);
-    pipe (fd);
+    if (fflush (stdout)); {
+      perror ("failed to flush standard output");
+      return -1;
+    }
+    if (pipe (fd) < 0) {
+      perror ("pipe failed");
+      return -1;
+    }
     pid = fork ();
     if (pid == -1) {
       perror ("fork");
@@ -760,7 +766,10 @@ issue_command (const char *cmd, char *argv[], const char
*pipecmd)

     if (pid == 0) {		/* Child process. */
       close (fd[1]);
-      dup2 (fd[0], 0);
+      if (dup2 (fd[0], 0) < 0) {
+        perror ("dup2 of stdin failed");
+        _exit (1);
+      }

       r = system (pipecmd);
       if (r == -1) {
@@ -770,9 +779,16 @@ issue_command (const char *cmd, char *argv[], const char
*pipecmd)
       _exit (WEXITSTATUS (r));
     }

-    stdout_saved_fd = dup (1);
+    if ((stdout_saved_fd = dup (1)) < 0) {
+      perror ("failed to dup stdout");
+      return -1;
+    }
     close (fd[0]);
-    dup2 (fd[1], 1);
+    if (dup2 (fd[1], 1) < 0) {
+      perror ("failed to dup stdout");
+      close (stdout_saved_fd);
+      return -1;
+    }
     close (fd[1]);
   }

--
1.6.4.378.g88f2f

Matthew Booth

2009-Aug-19 09:06 UTC

head link

[Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls

On 19/08/09 09:34, Jim Meyering wrote:> There were a few unchecked syscalls in fish.c
>
>> From ba8b8b0684a03b6e6fbb939ed7e1cbf5e1000092 Mon Sep 17 00:00:00 2001
> From: Jim Meyering<meyering at redhat.com>
> Date: Wed, 19 Aug 2009 10:01:07 +0200
> Subject: [PATCH libguestfs] guestfish: detect a few more failed syscalls
>
> * fish/fish.c (issue_command): Detect/diagnose more failed syscalls.
> ---
>   fish/fish.c |   26 +++++++++++++++++++++-----
>   1 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/fish/fish.c b/fish/fish.c
> index 830617b..e6cd270 100644
> --- a/fish/fish.c
> +++ b/fish/fish.c
> @@ -750,8 +750,14 @@ issue_command (const char *cmd, char *argv[], const
char *pipecmd)
>     if (pipecmd) {
>       int fd[2];
>
> -    fflush (stdout);
> -    pipe (fd);
> +    if (fflush (stdout)); {
Looks like a stray semicolon in there. Also, wouldn't it be better form 
to test for test for a return value of EOF?
> +      perror ("failed to flush standard output");
> +      return -1;
> +    }
> +    if (pipe (fd)) {
> +      perror ("pipe failed");
> +      return -1;
> +    }
Again, I'd explicitly test for a return value of -1 for consistency.
>       pid = fork ();
>       if (pid == -1) {
>         perror ("fork");
> @@ -760,7 +766,10 @@ issue_command (const char *cmd, char *argv[], const
char *pipecmd)
>
>       if (pid == 0) {		/* Child process. */
>         close (fd[1]);
> -      dup2 (fd[0], 0);
> +      if (dup2 (fd[0], 0)<  0) {
> +        perror ("dup2 of stdin failed");
> +        _exit (1);
> +      }
fd[1] ought to be closed after this call. Not directly relevant to this 
patch, but '0' is a magic number. Use STDIN_FILENO from unistd.h.
>         r = system (pipecmd);
>         if (r == -1) {
> @@ -770,9 +779,16 @@ issue_command (const char *cmd, char *argv[], const
char *pipecmd)
>         _exit (WEXITSTATUS (r));
>       }
>
> -    stdout_saved_fd = dup (1);
> +    if ((stdout_saved_fd = dup (1))<  0) {
> +      perror ("failed to dup stdout");
> +      return -1;
> +    }
'1' is a magic number. Use STDOUT_FILENO from unistd.h.
>       close (fd[0]);
> -    dup2 (fd[1], 1);
> +    if (dup2 (fd[1], 1)) {
> +      perror ("failed to dup stdout");
> +      close (stdout_saved_fd);
> +      return -1;
> +    }
Test for return value of -1. '1' is a magic number.
>       close (fd[1]);
>     }
>
> --
> 1.6.4.378.g88f2f
Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490

Maybe Matching Threads

Search for more apparently analagous threads

Libguestfs - Aug 2009 - [PATCH libguestfs] guestfish: detect a few more failed syscalls

[Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls

[Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls

[Libguestfs] [PATCH libguestfs] guestfish: detect a few more failed syscalls

Maybe Matching Threads