# HG changeset patch
# User Binki <ohnobinki@ohnopublishing.net>
# Date 2009-02-28 11:55:40
# Node ID 49749aac184e677f270da2e6497ca0637fb0939b
# Parent  966150c99fcc5e76210bba97a2536630e88f56c7

fixed execio's call to dup2, updated distren.c's test code

diff --git a/src/client/distren.c b/src/client/distren.c
--- a/src/client/distren.c
+++ b/src/client/distren.c
@@ -25,20 +25,36 @@
 
 int main(int argc, char *argv[])
 {
+  char buf[10];
   struct execio *testrem;
-  char *execargv[] = 
+  char *execargv[] =
     {
       "ssh",
       "protofusion.org",
       "sh",
       "-c",
-      "\"echo hello from ${HOSTNAME} 1>&2\"",
+      "\"echo hello from ${HOSTNAME}\"",
       (char *)NULL
     };
+  
+  size_t readlen;
+  
 
   fprintf(stderr, "testing execio (It shouldn't work) :-)\n");
+  fprintf(stderr, "execio_open returns %d\n", execio_open(&testrem, "ssh", execargv));
   
-  fprintf(stderr, "execio_open returns %d\n", execio_open(&testrem, "ssh", execargv));
+  buf[9] = '\0';
+  while(!execio_read(testrem, buf, 9, &readlen))
+    {
+      if(readlen > 9)
+	{
+	  fprintf(stderr, "execio_read doesn't set readlen correctly or read() is messed up\n");
+	  return 1;
+	}
+      buf[readlen] = '\0';
+      fprintf(stderr, "read \"%s\"\n", buf);
+    }
+  
 
   return 0;
 };
diff --git a/src/common/execio.c b/src/common/execio.c
--- a/src/common/execio.c
+++ b/src/common/execio.c
@@ -24,6 +24,7 @@
 #include <sys/wait.h>
 #include <signal.h>
 #include <malloc.h>
+#include <fcntl.h>
 #include <stdio.h>
 
 int execio_open(struct execio **rem, const char *progname, char *const argv[])
@@ -52,6 +53,9 @@ int execio_open(struct execio **rem, con
       return 1;
     }
   
+  /* make our side of the pipes nonblocking? I'll consider this type of stuff later and just keep this code in a comment like I always do... */
+  //fcntl(pipe_write[1], F_SETFD, fcntl(pipe_write[1], F_GETFD) | O_NONBLOCK);
+  //fucntl(pipe_read[0], F_SETFD, fcntl(pipe_read[0], F_GETFD) | O_NONBLOCK);
   
   /* parent */
   child = fork();
@@ -64,6 +68,7 @@ int execio_open(struct execio **rem, con
       return 1;
     }
   if(child)
+    /* the parent proc: */
     {
       /* close sides of pipe we won't use */
       close(pipe_write[0]);
@@ -100,8 +105,14 @@ int execio_open(struct execio **rem, con
       /*
 	reset stdin, stdout, and stderr to the appropriate files. OK, not stderr :-) 
       */
-      dup2(pipe_read[1], 0);
-      dup2(pipe_write[0], 1);
+      dup2(pipe_read[1], STDOUT_FILENO);
+      dup2(pipe_write[0], STDIN_FILENO);
+      /*
+	close the fds that were dup'd
+       */
+      close(pipe_read[1]);
+      close(pipe_write[0]);
+
       /* 
 	 close all other file descriptors. We want to keep 0, 1, and 2 open. We don't know that the last open() or pipe() always gives the highest fd number. However, I will assume that it does. Maybe this is a bad idea:
        */
@@ -134,10 +145,23 @@ int execio_open(struct execio **rem, con
     }
 }
 
-size_t execio_read(struct execio *eio, void *buf, size_t len)
+/*
+  returns 1 if child has exited, 
+  returns 0 if child is still alive
+ */
+int _execio_checkpid(struct execio *eio)
 {
-  int toreturn;
+  int childstatus;
+  
+  waitpid(eio->child, &childstatus, WNOHANG);
+  /* perror()? */
 
+  return WIFEXITED(childstatus);
+}
+
+
+int execio_read(struct execio *eio, void *buf, size_t len, size_t *bytesread)
+{
   /*
     TODO: detect NULL eio? 
     TODO: errno?
@@ -145,25 +169,27 @@ size_t execio_read(struct execio *eio, v
 
     whenever read() returns 0, it means EOF
    */
-  toreturn = read(eio->pipe_read, buf, len);
-  if(!toreturn)
-    /* should also be able to figure out if is bad fd and should set EXECIO_STATE_ERROR instead of _EOF */
-    eio->state = EXECIO_STATE_EOF;
+  (*bytesread) = read(eio->pipe_read, buf, len);
+  if(!*bytesread)
+    {
+      /* should also be able to figure out if is bad fd and should set EXECIO_STATE_ERROR instead of _EOF */
+      eio->state = EXECIO_STATE_EOF;
+      return 1;
+    }
 
-  return toreturn;
+  return 0;
 }
 
-size_t execio_write(struct execio *eio, void *buf, size_t len)
+int execio_write(struct execio *eio, void *buf, size_t len, size_t *bytesread)
 {
-  int toreturn;
   /*
     TODO: errno handling
    */
-  toreturn = write(eio->pipe_write, buf, len);
-  if(!toreturn)
+  (*bytesread) = write(eio->pipe_write, buf, len);
+  if(!*bytesread)
     eio->state = EXECIO_STATE_ERROR;
 
-   return toreturn;
+  return 0;
 }
 
 
@@ -175,9 +201,16 @@ enum execio_state execio_state(struct ex
 
 int execio_close(struct execio *eio)
 {
+  int childstatus;
+
   close(eio->pipe_read);
   close(eio->pipe_write);
 
+  /* maybe we should just kill the child */
+  kill(eio->child, SIGTERM);
+  /* the waitpid(2) seems to indicate that only when the child is terminated will this wait return. */
+  waitpid(eio->child, &childstatus, 0);
+  
   return 0;
 }
 
diff --git a/src/common/execio.h b/src/common/execio.h
--- a/src/common/execio.h
+++ b/src/common/execio.h
@@ -51,11 +51,11 @@ struct execio
 int execio_open(struct execio **rem, const char *progname, char *const argv[]);
 
 /* 
-   blocks, 
-   returns 0 if len is 0. Otherwise, only returns 0 on error/EOF: use execio_state() to determine
+   doesn't block, 
+   returns 0 on success, 1 on failure
 */
-size_t execio_read(struct execio *eio, void *buf, size_t len);
-size_t execio_write(struct execio *eio, void *buf, size_t len);
+int execio_read(struct execio *eio, void *buf, size_t len, size_t *bytesread);
+int execio_write(struct execio *eio, void *buf, size_t len, size_t *bytesread);
 
 enum execio_state execio_state(struct execio *eio);