From: Peter Stephenson <p.w.stephenson@ntlworld.com>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: zsh-workers@zsh.org
Subject: Re: Mangement of fdtable[]
Date: Sat, 24 Oct 2015 20:14:39 +0100 [thread overview]
Message-ID: <20151024201439.35cb8bf1@ntlworld.com> (raw)
In-Reply-To: <151024114352.ZM27626@torch.brasslantern.com>
On Sat, 24 Oct 2015 11:43:52 -0700
Bart Schaefer <schaefer@brasslantern.com> 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 */
next prev parent reply other threads:[~2015-10-24 19:14 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-16 0:22 Bart Schaefer
2015-10-16 9:56 ` Chi Hsuan Yen
2015-10-16 19:44 ` Peter Stephenson
2015-10-24 18:16 ` Peter Stephenson
2015-10-24 18:43 ` Bart Schaefer
2015-10-24 19:14 ` Peter Stephenson [this message]
2015-10-24 19:37 ` Bart Schaefer
2015-10-24 19:43 ` Peter Stephenson
2015-10-24 21:05 ` Bart Schaefer
2015-10-25 18:44 ` Peter Stephenson
2015-10-26 1:15 ` Bart Schaefer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151024201439.35cb8bf1@ntlworld.com \
--to=p.w.stephenson@ntlworld.com \
--cc=schaefer@brasslantern.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).