From: ian <ian>
Date: Sun, 29 Jun 2003 12:52:14 +0000 (+0000)
Subject: @@ -1,5 +1,7 @@
X-Git-Tag: debian_version_1_0_1_99_0~7
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?p=userv.git;a=commitdiff_plain;h=7d720638764f8a58d122e5ee1c9e5cbb1497d987

@@ -1,5 +1,7 @@
+  * Make require-fd work with reading fds !
+    (Thanks to Ben Harris for the bug report).
---

diff --git a/debian/changelog b/debian/changelog
index 248f460..7ba2faa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,5 +1,7 @@
 userv (1.0.2) unstable; urgency=low
 
+  * Make require-fd work with reading fds !
+    (Thanks to Ben Harris for the bug report).
   * Specification's usage notes section improved.
   * --help and --version behaviour made to conform to GNU standards.
   * userv(1) manpage: fixed broken definitions of fd excl and trunc.
diff --git a/process.c b/process.c
index c4a4809..1d5963d 100644
--- a/process.c
+++ b/process.c
@@ -443,6 +443,8 @@ static void receive_request(void) {
     assert(fdarray[fd].iswrite == -1);
     fdarray[fd].iswrite= (i>=request_mbuf.nreadfds);
   }
+  /* fdarray[].iswrite now set; rest is still blank
+   * (ie want reject read, no realfd holdfd). */
 
   assert(request_mbuf.nargs <= MAX_ARGSDEFVAR);
   argarray= xmalloc(sizeof(char*)*(request_mbuf.nargs));
@@ -482,6 +484,14 @@ static void establish_pipes(void) {
     if (unlink(pipepathbuf)) syscallerror("unlink pipe");
     if (close(tempfd)) syscallerror("close prelim fd onto pipe");
   }
+  /* Now fdarray[].realfd is pipe end for service in case service
+   * wants it.  If it's an input pipe, then .holdfd is the other
+   * (writing) end of the pipe - we keep it around so that the service
+   * doesn't get an apparently clean EOF if the caller disappears (eg
+   * due to a file read error) or the like (ie so that on disconnect
+   * we can guarantee to send the service SIGHUP before it gets EOF on
+   * the input fd).  Otherwise, .holdfd=-1.
+   */
 }
 
 static void groupnames(int ngids, gid_t *gids, const char ***names_r) {
@@ -645,7 +655,6 @@ static void check_fds(void) {
     case tokv_word_requirefd:
       if (fdarray[fd].realfd == -1)
 	failure("file descriptor %d required but not provided",fd);
-      assert(fdarray[fd].holdfd == -1);
       /* fall through */
     case tokv_word_allowfd:
       if (fdarray[fd].realfd == -1) {
@@ -662,6 +671,10 @@ static void check_fds(void) {
       }
     }
   }
+  /* Now fdarray[].realfd is exactly what service wants: pipe end or
+   * /dev/null or -1.  If .realfd is not -1 then .holdfd may be the fd
+   * for the writing end of the corresponding pipe.
+   */
 }
 
 static void send_progress_ok(void) {
diff --git a/servexec.c b/servexec.c
index 8c3494a..aa35fc7 100644
--- a/servexec.c
+++ b/servexec.c
@@ -250,7 +250,16 @@ void execservice(const int synchsocket[], int clientfd) {
 
   if (close(clientfd)) serv_syscallfail("close client socket fd");
 
-  /* Now we have to make all the fd's work.  It's rather a complicated
+  /* First we need to close the holding writing ends of the pipes
+   * inherited from our parent: */
+  for (fd=0; fd<fdarrayused; fd++) {
+    if (fdarray[fd].holdfd == -1) continue;
+    if (close(fdarray[fd].holdfd)) serv_syscallfail("close pipe hold fd");
+    fdarray[fd].holdfd= -1;
+  }
+  /* Now we can reuse the .holdfd member of the fdarray entries. */
+
+  /* We have to make all the fd's work.  It's rather a complicated
    * algorithm, unfortunately.  We remember in holdfd[fd] whether fd
    * is being used to hold a file descriptor we actually want for some
    * other real fd in the service program; holdfd[fd] contains the fd
@@ -260,11 +269,6 @@ void execservice(const int synchsocket[], int clientfd) {
    * holdfd tells us we're currently storing some other fd in there we
    * move it out of the way with dup and record its new location.
    */
-  for (fd=0; fd<fdarrayused; fd++) {
-    if (fdarray[fd].holdfd == -1) continue;
-    if (close(fdarray[fd].holdfd)) serv_syscallfail("close pipe hold fd");
-    fdarray[fd].holdfd= -1;
-  }
   for (fd=0; fd<fdarrayused; fd++) {
     if (fdarray[fd].realfd < fdarrayused && fdarray[fd].realfd >= 0)
       fdarray[fdarray[fd].realfd].holdfd= fd;