Re: Mangement of fdtable[]

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

On Sat, 24 Oct 2015 11:43:52 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Oct 24,  7:16pm, Peter Stephenson wrote:
> } Subject: Re: Mangement of fdtable[]
> }
> } Is this the fix for tcp.c (not socket.c)?
> 
> This looks very similar to what I was contemplating, except that I had
> not yet thought as far as adding another FDT_* flag.  The only other
> thing I was wondering about is allowing the module to control whether
> the FD is closed when entering a subshell [i.e., when exec.c calls
> closem(FDT_INTERNAL)], because there is not always an execv() in that
> case so simply setting CLOEXEC in the module is not enough.
> 
> } I didn't think FDT_INTERNAL was the right thing, on balance,
> } as they're too public for the shell taking over management
> 
> That's true for tcp.c, but perhaps not so for e.g. db_gdbm.c.

So that means addmodulefd() should have an additional argument saying if
the fd is to be treated as internal.

I don't want to add a different category saying it can be closed on
exec but not by standard shell syntax since I'll get that wrong.

diff --git a/Src/Modules/tcp.c b/Src/Modules/tcp.c
index bc1765d..274f01f 100644
--- a/Src/Modules/tcp.c
+++ b/Src/Modules/tcp.c
@@ -236,6 +236,8 @@ tcp_socket(int domain, int type, int protocol, int ztflags)
     if (!sess) return NULL;
 
     sess->fd = socket(domain, type, protocol);
+    /* We'll check failure and tidy up in caller */
+    addmodulefd(sess->fd, FALSE);
     return sess;
 }
 
@@ -298,7 +300,7 @@ tcp_close(Tcp_session sess)
     {  
 	if (sess->fd != -1)
 	{
-	    err = close(sess->fd);
+	    err = zclose(sess->fd);
 	    if (err)
 		zwarn("connection close failed: %e", errno);
 	}
@@ -546,6 +548,9 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
+	/* redup expects fd is already registered */
+	addmodulefd(rfd, FALSE);
+
 	if (targetfd) {
 	    sess->fd = redup(rfd, targetfd);
 	    if (sess->fd < 0) {
diff --git a/Src/utils.c b/Src/utils.c
index 61cbe84..4c69d75 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1892,6 +1892,32 @@ 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).
+ *
+ * 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)
+{
+    if (fd >= 0) {
+	check_fd_table(fd);
+	fdtable[fd] = is_internal ? FDT_INTERNAL : FDT_MODULE;
+    }
+}
+
+/**/
+
+/*
  * Indicate that an fd has a file lock; if cloexec is 1 it will be closed
  * on exec.
  * The fd should already be known to fdtable (e.g. by movefd).
diff --git a/Src/zsh.h b/Src/zsh.h
index 15fa5e4..f819249 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -406,25 +406,32 @@ enum {
  */
 #define FDT_EXTERNAL		2
 /*
+ * Entry visible to other processes but controlled by a module.
+ * The difference from FDT_EXTERNAL is that closing this using
+ * standard fd syntax will fail as there is some tidying up that
+ * needs to be done by the module's own mechanism.
+ */
+#define FDT_MODULE		3
+/*
  * Entry used by output from the XTRACE option.
  */
-#define FDT_XTRACE		3
+#define FDT_XTRACE		4
 /*
  * Entry used for file locking.
  */
-#define FDT_FLOCK		4
+#define FDT_FLOCK		5
 /*
  * As above, but the fd is not marked for closing on exec,
  * so the shell can still exec the last process.
  */
-#define FDT_FLOCK_EXEC		5
+#define FDT_FLOCK_EXEC		6
 #ifdef PATH_DEV_FD
 /*
  * Entry used by a process substition.
  * This marker is not tested internally as we associated the file
  * descriptor with a job for closing.
  */
-#define FDT_PROC_SUBST		6
+#define FDT_PROC_SUBST		7
 #endif
 
 /* Flags for input stack */