Re: [PATCH] compinit forks less

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Thu, Apr 30, 2026 at 11:29 AM Charles Blake <charlechaud@xxxxxxxxx> 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.

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))))