Re: [PATCH] compinit forks less
Bart Schaefer <[email protected]>
| Newsgroups | gmane.comp.shells.zsh.devel |
|---|---|
| Message-ID | <CAH+w=7aTJV-QcBtmWy=9_AmL=e21rKqn3n35qo6daXb+dk1N8w@mail.gmail.com> |
On Thu, Apr 30, 2026 at 11:29 AM Charles Blake <[email protected]> wrote: > > It bears mention that while my first here-string idea uses two temp > files, this new still uses one temp file, and a racy one at that (not > that I see any real attack by messing up bindkey introspection). > I'm not sure where you're coming up with "racy". The same mechanism is used for this file as is used for =(...) substitutions. If there's a problem here, there's a problem there. I imagine this must have been discussed long ago when ${ } got in > This is essentially the same way bash also implements ${ ... }. There's no portable deadlock-free way to have a process connect to itself with a pipe. This was discussed when the feature was developed. All that said, there's a different way to go about it, see attached. Question, should rplyoutfd here be entered into fdtable somehow? It is not for here-strings.
nofork_safety.txt
(text/plain, 1.6 KB)
diff --git a/Src/subst.c b/Src/subst.c
index 56c1ad6dd..659074568 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1872,6 +1872,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
*/
char *rplyvar = NULL; /* Indicates ${|...;} or ${{var} ...;} */
char *rplytmp = NULL; /* Indicates ${ ... ;} */
+ int rplyoutfd;
*s++ = '\0';
/*
@@ -1990,14 +1991,15 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
* Then fall through to the regular parameter handling
* to manage word splitting, expansion flags, etc.
*/
- char *outfmt = ">| %s {\n%s\n;}"; /* 13 */
- if ((rplytmp = gettempname(NULL, 1))) {
- /* Prevent shenanigans with $TMPPREFIX */
- char *tmpfile = quotestring(rplytmp, QT_BACKSLASH);
- char *dummy = zhalloc(strlen(cmdarg) +
- strlen(tmpfile) +
- 13);
- sprintf(dummy, outfmt, tmpfile, cmdarg);
+ char *outfmt = ">&%d {\n%s\n;}"; /* 13 */
+ if ((rplyoutfd = gettempfile(NULL, 1, &rplytmp))) {
+ char *dummy = zhalloc(strlen(cmdarg) + 20);
+#if defined(F_SETFD) && defined(FD_CLOEXEC)
+ /* If cmdarg execs a program, close this FD. */
+ fcntl(rplyoutfd, F_SETFD, FD_CLOEXEC);
+#endif
+
+ sprintf(dummy, outfmt, rplyoutfd, cmdarg);
cmdarg = dummy;
} else {
/* TMPPREFIX not writable? */
@@ -2074,8 +2076,10 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
unqueue_signals();
}
- if (rplytmp)
+ if (rplytmp) {
+ zclose(rplyoutfd);
unlink(rplytmp);
+ }
if (rplyvar) {
if (inchar != Inbrace) {
if ((val = dupstring(getsparam(rplyvar))))