From 1f6dcf9bcdade41886e36826196e79502a9afa26 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Sat, 24 Oct 2015 20:24:35 +0100 Subject: 36941: Mark file descripors in ztcp as used. Allow such file descriptors to be either internal and closed on exec or external and so managed explicitly by module. --- Src/Modules/tcp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'Src/Modules/tcp.c') diff --git a/Src/Modules/tcp.c b/Src/Modules/tcp.c index bc1765da1..274f01ffc 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) { -- cgit v1.2.3 From 15490398d7553605c422d8ab0e59e69e25111df4 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Sat, 24 Oct 2015 20:48:47 +0100 Subject: 36944: extend fd management to zsocket --- ChangeLog | 4 ++++ Doc/Zsh/mod_socket.yo | 5 +++++ Src/Modules/socket.c | 17 +++++++++++++++-- Src/Modules/tcp.c | 4 ++-- Src/utils.c | 25 +++++++++++++++---------- 5 files changed, 41 insertions(+), 14 deletions(-) (limited to 'Src/Modules/tcp.c') diff --git a/ChangeLog b/ChangeLog index 356e92319..71f15bb0d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2015-10-24 Peter Stephenson + * 36944: Src/utils.c, Src/Modules/tcp.c, Src/Modules/socket.c: + extend the previous to zsocket, although this needs to allow the + fd to be closed explicitly. + * 36941: Src/utils.c, Src/zsh.h, Src/Modules/tcp.c: ensure shell knows about file descriptiors used by ztcp and allow such file descriptors to be marked as internal if needed (not yet used). diff --git a/Doc/Zsh/mod_socket.yo b/Doc/Zsh/mod_socket.yo index 332c551fe..867f6081f 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 65b87d7dd..f683496df 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 274f01ffc..7b0dcc74a 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 4c69d75a1..37a02c593 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; } } -- cgit v1.2.3 From 0628802baf5c9245138db82dd058cad023a7d0ae Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Thu, 29 Oct 2015 15:01:07 +0000 Subject: 37014: Improved internal parameter setting. Enhance WARNCREATEGLOBAL to work in many more cases. Don't create REPLY as an integer if it didn't previously exist as one, even if the value to be set is integral, as this is likely to mess up later uses of REPLY. --- ChangeLog | 11 +++++++++ Functions/MIME/zsh-mime-setup | 2 +- Functions/Misc/add-zsh-hook | 2 ++ Src/Modules/socket.c | 6 ++--- Src/Modules/tcp.c | 6 ++--- Src/Modules/zpty.c | 2 +- Src/builtin.c | 38 ++++++++++++++++++------------- Src/params.c | 53 +++++++++++++++++++++++++++++++++++++++++-- Src/zsh.h | 3 --- 9 files changed, 94 insertions(+), 29 deletions(-) (limited to 'Src/Modules/tcp.c') diff --git a/ChangeLog b/ChangeLog index 0c38b2697..9454013cb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2015-10-29 Peter Stephenson + + * 37014: Functions/MIME/zsh-mime-setup, + Functions/Misc/add-zsh-hook, Src/Modules/socket.c, + Src/Modules/tcp.c, Src/Modules/zpty.c, Src/builtin.c, + Src/params.c,Src/zsh.h: improved internal parameter setting. + Enhance WARNCREATEGLOBAL to work in many more cases. Don't + create REPLY as an integer if it didn't previously exist as one, + even if the value to be set is integral, as this is likely to + mess up later uses of REPLY. + 2015-10-29 Jun-ichi Takimoto * 36983 (with fix from 36990): Completion/Unix/Command/_head, diff --git a/Functions/MIME/zsh-mime-setup b/Functions/MIME/zsh-mime-setup index 23e44fdc0..35f6e6b6b 100644 --- a/Functions/MIME/zsh-mime-setup +++ b/Functions/MIME/zsh-mime-setup @@ -1,7 +1,7 @@ emulate -L zsh setopt extendedglob cbases -local opt o_verbose o_list +local opt o_verbose o_list i autoload -Uz zsh-mime-handler diff --git a/Functions/Misc/add-zsh-hook b/Functions/Misc/add-zsh-hook index ee37d674d..fc39659ae 100644 --- a/Functions/Misc/add-zsh-hook +++ b/Functions/Misc/add-zsh-hook @@ -82,9 +82,11 @@ if (( del )); then else if (( ${(P)+hook} )); then if (( ${${(P)hook}[(I)$fn]} == 0 )); then + typeset -ga $hook set -A $hook ${(P)hook} $fn fi else + typeset -ga $hook set -A $hook $fn fi autoload $autoopts -- $fn diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c index f683496df..7c3fb5ebe 100644 --- a/Src/Modules/socket.c +++ b/Src/Modules/socket.c @@ -132,7 +132,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func)) /* allow to be closed explicitly */ fdtable[sfd] = FDT_EXTERNAL; - setiparam("REPLY", sfd); + setiparam_no_convert("REPLY", (zlong)sfd); if (verbose) printf("%s listener is on fd %d\n", soun.sun_path, sfd); @@ -220,7 +220,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func)) sfd = rfd; } - setiparam("REPLY", sfd); + setiparam_no_convert("REPLY", (zlong)sfd); if (verbose) printf("new connection from %s is on fd %d\n", soun.sun_path, sfd); @@ -261,7 +261,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func)) fdtable[sfd] = FDT_EXTERNAL; } - setiparam("REPLY", sfd); + setiparam_no_convert("REPLY", (zlong)sfd); if (verbose) printf("%s is now on fd %d\n", soun.sun_path, sfd); diff --git a/Src/Modules/tcp.c b/Src/Modules/tcp.c index 7b0dcc74a..9fc1b29a2 100644 --- a/Src/Modules/tcp.c +++ b/Src/Modules/tcp.c @@ -461,7 +461,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func)) return 1; } - setiparam("REPLY", sess->fd); + setiparam_no_convert("REPLY", (zlong)sess->fd); if (verbose) printf("%d listener is on fd %d\n", ntohs(sess->sock.in.sin_port), sess->fd); @@ -562,7 +562,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func)) sess->fd = rfd; } - setiparam("REPLY", sess->fd); + setiparam_no_convert("REPLY", (zlong)sess->fd); if (verbose) printf("%d is on fd %d\n", ntohs(sess->peer.in.sin_port), sess->fd); @@ -681,7 +681,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func)) } } - setiparam("REPLY", sess->fd); + setiparam_no_convert("REPLY", (zlong)sess->fd); if (verbose) printf("%s:%d is now on fd %d\n", diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c index 9741ee287..3b8366076 100644 --- a/Src/Modules/zpty.c +++ b/Src/Modules/zpty.c @@ -464,7 +464,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock) #endif errno == EINTR)); - setiparam("REPLY", master); + setiparam_no_convert("REPLY", (zlong)master); return 0; } diff --git a/Src/builtin.c b/Src/builtin.c index 97022addf..8045bc8f7 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -2090,7 +2090,9 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), tc = 0; /* but don't do a normal conversion */ } } else if (!setsecondstype(pm, on, off)) { - if (asg->value.scalar && !(pm = setsparam(pname, ztrdup(asg->value.scalar)))) + if (asg->value.scalar && + !(pm = assignsparam( + pname, ztrdup(asg->value.scalar), 0))) return NULL; usepm = 1; err = 0; @@ -2202,12 +2204,13 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), } else if (pm->env && !(pm->node.flags & PM_HASHELEM)) delenv(pm); DPUTS(ASG_ARRAYP(asg), "BUG: typeset got array value where scalar expected"); - if (asg->value.scalar && !(pm = setsparam(pname, ztrdup(asg->value.scalar)))) + if (asg->value.scalar && + !(pm = assignsparam(pname, ztrdup(asg->value.scalar), 0))) return NULL; } else if (asg->is_array) { - if (!(pm = setaparam(pname, asg->value.array ? + if (!(pm = assignaparam(pname, asg->value.array ? zlinklist2array(asg->value.array) : - mkarray(NULL)))) + mkarray(NULL), 0))) return NULL; } pm->node.flags |= (on & PM_READONLY); @@ -2347,16 +2350,18 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), * creating a stray parameter along the way via createparam(), * now called below in the isident() branch. */ - if (!(pm = setsparam(pname, ztrdup(asg->value.scalar ? asg->value.scalar : "")))) + if (!(pm = assignsparam( + pname, + ztrdup(asg->value.scalar ? asg->value.scalar : ""), 0))) return NULL; dont_set = 1; asg->is_array = 0; keeplocal = 0; on = pm->node.flags; } else if (PM_TYPE(on) == PM_ARRAY && ASG_ARRAYP(asg)) { - if (!(pm = setaparam(pname, asg->value.array ? - zlinklist2array(asg->value.array) : - mkarray(NULL)))) + if (!(pm = assignaparam(pname, asg->value.array ? + zlinklist2array(asg->value.array) : + mkarray(NULL), 0))) return NULL; dont_set = 1; keeplocal = 0; @@ -2433,13 +2438,13 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), Param ipm = pm; if (pm->node.flags & (PM_ARRAY|PM_HASHED)) { DPUTS(!ASG_ARRAYP(asg), "BUG: inconsistent scalar value for array"); - if (!(pm=setaparam(pname, asg->value.array ? - zlinklist2array(asg->value.array) : - mkarray(NULL)))) + if (!(pm=assignaparam(pname, asg->value.array ? + zlinklist2array(asg->value.array) : + mkarray(NULL), 0))) return NULL; } else { DPUTS(ASG_ARRAYP(asg), "BUG: inconsistent array value for scalar"); - if (!(pm = setsparam(pname, ztrdup(asg->value.scalar)))) + if (!(pm = assignsparam(pname, ztrdup(asg->value.scalar), 0))) return NULL; } if (pm != ipm) { @@ -2687,9 +2692,10 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func) /* Update join character */ tdp->joinchar = joinchar; if (asg0.value.scalar) - setsparam(asg0.name, ztrdup(asg0.value.scalar)); + assignsparam(asg0.name, ztrdup(asg0.value.scalar), 0); else if (asg->value.array) - setaparam(asg->name, zlinklist2array(asg->value.array)); + assignaparam( + asg->name, zlinklist2array(asg->value.array), 0); return 0; } else { zwarnnam(name, "can't tie already tied scalar: %s", @@ -2750,9 +2756,9 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func) zsfree(apm->ename); apm->ename = ztrdup(asg0.name); if (asg->value.array) - setaparam(asg->name, zlinklist2array(asg->value.array)); + assignaparam(asg->name, zlinklist2array(asg->value.array), 0); else if (oldval) - setsparam(asg0.name, oldval); + assignsparam(asg0.name, oldval, 0); unqueue_signals(); return 0; diff --git a/Src/params.c b/Src/params.c index a8abb289e..4d33660fb 100644 --- a/Src/params.c +++ b/Src/params.c @@ -2826,6 +2826,15 @@ assignsparam(char *s, char *val, int flags) return v->pm; } +/**/ +mod_export Param +setsparam(char *s, char *val) +{ + return assignsparam( + s, val, isset(WARNCREATEGLOBAL) && locallevel > 0 ? + ASSPM_WARN_CREATE : 0); +} + /**/ mod_export Param assignaparam(char *s, char **val, int flags) @@ -2914,6 +2923,16 @@ assignaparam(char *s, char **val, int flags) return v->pm; } + +/**/ +mod_export Param +setaparam(char *s, char **aval) +{ + return assignaparam( + s, aval, isset(WARNCREATEGLOBAL) && locallevel >0 ? + ASSPM_WARN_CREATE : 0); +} + /**/ mod_export Param sethparam(char *s, char **val) @@ -2937,11 +2956,15 @@ sethparam(char *s, char **val) if (unset(EXECOPT)) return NULL; queue_signals(); - if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) + if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) { createparam(t, PM_HASHED); - else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) && + if (isset(WARNCREATEGLOBAL) && locallevel > 0 && v->pm->level == 0) + zwarn("associative array parameter %s created globally in function", + v->pm->node.nam); + } else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) && !(v->pm->node.flags & PM_SPECIAL)) { unsetparam(t); + /* no WARNCREATEGLOBAL check here as parameter already existed */ createparam(t, PM_HASHED); v = NULL; } @@ -2968,6 +2991,7 @@ setnparam(char *s, mnumber val) Value v; char *t = s, *ss; Param pm; + int was_unset = 0; if (!isident(s)) { zerr("not an identifier: %s", s); @@ -2987,6 +3011,7 @@ setnparam(char *s, mnumber val) */ unset(KSHARRAYS) && !ss) { unsetparam_pm(v->pm, 0, 1); + was_unset = 1; s = t; v = NULL; } @@ -3007,6 +3032,10 @@ setnparam(char *s, mnumber val) } v = getvalue(&vbuf, &t, 1); DPUTS(!v, "BUG: value not found for new parameter"); + if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0 && + v->pm->level == 0) + zwarn("numeric parameter %s created globally in function", + v->pm->node.nam); } setnumvalue(v, val); unqueue_signals(); @@ -3025,6 +3054,26 @@ setiparam(char *s, zlong val) return setnparam(s, mnval); } +/* + * Set an integer parameter without forcing creation of an integer type. + * This is useful if the integer is going to be set to a parmaeter which + * would usually be scalar but may not exist. + */ + +/**/ +mod_export Param +setiparam_no_convert(char *s, zlong val) +{ + /* + * If the target is already an integer, thisgets converted + * back. Low technology rules. + */ + char buf[BDIGBUFSIZE]; + convbase(buf, val, 10); + return assignsparam( + s, ztrdup(buf), + isset(WARNCREATEGLOBAL) && locallevel > 0 ? ASSPM_WARN_CREATE : 0); +} /* Unset a parameter */ diff --git a/Src/zsh.h b/Src/zsh.h index f819249c3..d03d171e4 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -1939,9 +1939,6 @@ struct paramdef { { name, flags | PM_SPECIAL | PM_HIDE | PM_HIDEVAL, \ NULL, gsufn, getfn, scanfn, NULL } -#define setsparam(S,V) assignsparam(S,V,0) -#define setaparam(S,V) assignaparam(S,V,0) - /* * Flags for assignsparam and assignaparam. */ -- cgit v1.2.3