Jim Meyering wrote:
 Matthew Booth wrote:
> 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(a)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?
 Argh.  Thanks.
 And yes, testing for EOF does seem better.
 I also noticed that there are other unchecked dup2 and fflush calls.
 Another round coming right up. 
And waitpid.
This compiles.
"make check" in progress.
From f0e6f60d95296024de57a0970f151023f9196df3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Wed, 19 Aug 2009 10:01:07 +0200
Subject: [PATCH libguestfs] guestfish: detect a more failed syscalls
* fish/fish.c (issue_command): Detect/diagnose more failed syscalls.
---
 fish/fish.c |   41 +++++++++++++++++++++++++++++++++--------
 1 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/fish/fish.c b/fish/fish.c
index 830617b..987df59 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) == EOF) {
+      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]);
   }
@@ -823,13 +839,22 @@ issue_command (const char *cmd, char *argv[], const char *pipecmd)
   /* Always flush stdout after every command, so that messages, results
    * etc appear immediately.
    */
-  fflush (stdout);
+  if (fflush (stdout) == EOF) {
+    perror ("failed to flush standard output");
+    return -1;
+  }
   if (pipecmd) {
     close (1);
-    dup2 (stdout_saved_fd, 1);
+    if (dup2 (stdout_saved_fd, 1) < 0) {
+      perror ("failed to dup2 standard output");
+      r = -1;
+    }
     close (stdout_saved_fd);
-    waitpid (pid, NULL, 0);
+    if (waitpid (pid, NULL, 0) < 0) {
+      perror ("waiting for command to complete");
+      r = -1;
+    }
   }
   return r;
--
1.6.4.378.g88f2f