Re: Mangement of fdtable[]

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Sat, 24 Oct 2015 20:14:39 +0100
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> So that means addmodulefd() should have an additional argument saying if
> the fd is to be treated as internal.

That's not good enough.  For zsocket, it needs to be an FDT_EXTERNAL
because we want the user to determine whne it gets closed just the same
as with ztcp, but in this case it can only be closed with standard shell
syntax --- I've clarified this.

pws

diff --git a/Doc/Zsh/mod_socket.yo b/Doc/Zsh/mod_socket.yo
index 332c551..867f608 100644
--- a/Doc/Zsh/mod_socket.yo
+++ b/Doc/Zsh/mod_socket.yo
@@ -28,6 +28,11 @@ will be taken as the target file descriptor for the
 connection.
 
 In order to elicit more verbose output, use tt(-v).
+
+File descriptors can be closed with normal shell syntax when no longer
+needed, for example:
+
+example(exec {REPLY}>&-)
 )
 enditem()
 
diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c
index 65b87d7..f683496 100644
--- a/Src/Modules/socket.c
+++ b/Src/Modules/socket.c
@@ -115,6 +115,8 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
+	addmodulefd(sfd, FDT_EXTERNAL);
+
 	if (targetfd) {
 	    sfd = redup(sfd, targetfd);
 	}
@@ -127,6 +129,9 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
+	/* allow to be closed explicitly */
+	fdtable[sfd] = FDT_EXTERNAL;
+
 	setiparam("REPLY", sfd);
 
 	if (verbose)
@@ -200,12 +205,16 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
+	addmodulefd(rfd, FDT_EXTERNAL);
+
 	if (targetfd) {
 	    sfd = redup(rfd, targetfd);
 	    if (sfd < 0) {
 		zerrnam(nam, "could not duplicate socket fd to %d: %e", targetfd, errno);
+		zclose(rfd);
 		return 1;
 	    }
+	    fdtable[sfd] = FDT_EXTERNAL;
 	}
 	else {
 	    sfd = rfd;
@@ -240,12 +249,16 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	}
 	else
 	{
+	    addmodulefd(sfd, FDT_EXTERNAL);
+
 	    if (targetfd) {
-		sfd = redup(sfd, targetfd);
-		if (sfd < 0) {
+		if (redup(sfd, targetfd) < 0) {
 		    zerrnam(nam, "could not duplicate socket fd to %d: %e", targetfd, errno);
+		    zclose(sfd);
 		    return 1;
 		}
+		sfd = targetfd;
+		fdtable[sfd] = FDT_EXTERNAL;
 	    }
 
 	    setiparam("REPLY", sfd);
diff --git a/Src/Modules/tcp.c b/Src/Modules/tcp.c
index 274f01f..7b0dcc7 100644
--- a/Src/Modules/tcp.c
+++ b/Src/Modules/tcp.c
@@ -237,7 +237,7 @@ tcp_socket(int domain, int type, int protocol, int ztflags)
 
     sess->fd = socket(domain, type, protocol);
     /* We'll check failure and tidy up in caller */
-    addmodulefd(sess->fd, FALSE);
+    addmodulefd(sess->fd, FDT_MODULE);
     return sess;
 }
 
@@ -549,7 +549,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func))
 	}
 
 	/* redup expects fd is already registered */
-	addmodulefd(rfd, FALSE);
+	addmodulefd(rfd, FDT_MODULE);
 
 	if (targetfd) {
 	    sess->fd = redup(rfd, targetfd);
diff --git a/Src/utils.c b/Src/utils.c
index 4c69d75..37a02c5 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1894,24 +1894,29 @@ redup(int x, int y)
 /*
  * Add an fd opened ithin a module.
  *
- * If is_internal is FALSE, the fd can be used within the shell for
- * normal I/O but it will not be closed automatically or by normal shell
- * syntax; it can only be closed by the module (which should included
- * zclose() as part of the sequence).
+ * fdt is the type of the fd; see the FDT_ definitions in zsh.h.
+ * The most likely falures are:
+ *
+ * FDT_EXTERNAL: the fd can be used within the shell for normal I/O but
+ * it will not be closed automatically or by normal shell syntax.
+ *
+ * FDT_MODULE: as FDT_EXTERNAL, but it can only be closed by the module
+ * (which should included zclose() as part of the sequence, not by
+ * the standard shell syntax for closing file descriptors.
+ *
+ * FDT_INTERNAL: fd is treated like others created by the shell for
+ * internall use; it can be closed and will be closed by the shell if it
+ * exec's or performs an exec with a fork optimised out.
  *
- * If is_internal is TRUE the fd is treated like others created by the
- * shell for internall use; it can be closed and will be closed by the
- * shell if it exec's or performs an exec with a fork optimised out.
- *.
  * Safe if fd is -1 to indicate failure.
  */
 /**/
 mod_export void
-addmodulefd(int fd, bool is_internal)
+addmodulefd(int fd, int fdt)
 {
     if (fd >= 0) {
 	check_fd_table(fd);
-	fdtable[fd] = is_internal ? FDT_INTERNAL : FDT_MODULE;
+	fdtable[fd] = fdt;
     }
 }