From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19769 invoked by alias); 24 Oct 2015 19:14:47 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 36941 Received: (qmail 7107 invoked from network); 24 Oct 2015 19:14:44 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.0 X-Originating-IP: [86.6.158.222] X-Spam: 0 X-Authority: v=2.1 cv=UKUgZ3ry c=1 sm=1 tr=0 a=2SBOh4l1h08DI0L+aujZyQ==:117 a=2SBOh4l1h08DI0L+aujZyQ==:17 a=NLZqzBF-AAAA:8 a=kj9zAlcOel0A:10 a=q2GGsy2AAAAA:8 a=wJNOdiYg3O-jtxXNi2wA:9 a=CjuIK1q_8ugA:10 Date: Sat, 24 Oct 2015 20:14:39 +0100 From: Peter Stephenson To: Bart Schaefer Cc: zsh-workers@zsh.org Subject: Re: Mangement of fdtable[] Message-ID: <20151024201439.35cb8bf1@ntlworld.com> In-Reply-To: <151024114352.ZM27626@torch.brasslantern.com> References: <151015172252.ZM30709@torch.brasslantern.com> <20151024191621.7f6d71bf@ntlworld.com> <151024114352.ZM27626@torch.brasslantern.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Sat, 24 Oct 2015 11:43:52 -0700 Bart Schaefer 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 */