From 76ede1996b9d1e4317710be3a9669c51582c298b Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 27 Oct 2023 22:21:09 +0000 Subject: spawn: avoid alloca in C pi_fork_exec We don't have thread-safety to worry about, so just leave a few allocations at process exit at worst. We'll also update some comments about usage while we're at it. --- lib/PublicInbox/Spawn.pm | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'lib/PublicInbox/Spawn.pm') diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm index 5740ee58..8382c4d2 100644 --- a/lib/PublicInbox/Spawn.pm +++ b/lib/PublicInbox/Spawn.pm @@ -6,10 +6,8 @@ # is explicitly defined in the environment (and writable). # Under Linux, vfork can make a big difference in spawning performance # as process size increases (fork still needs to mark pages for CoW use). -# Currently, we only use this for code intended for long running -# daemons (inside the PSGI code (-httpd) and -nntpd). The short-lived -# scripts (-mda, -index, -learn, -init) either use IPC::run or standard -# Perl routines. +# None of this is intended to be thread-safe since Perl5 maintainers +# officially discourage the use of threads. # # There'll probably be more OS-level C stuff here, down the line. # We don't want too many DSOs: https://udrepper.livejournal.com/8790.html @@ -39,12 +37,6 @@ BEGIN { #include #include #include - -/* some platforms need alloca.h, but some don't */ -#if defined(__GNUC__) && !defined(alloca) -# define alloca(sz) __builtin_alloca(sz) -#endif - #include #include @@ -56,11 +48,17 @@ BEGIN { * This is unlike "sv_len", which returns what you would expect. */ #define AV2C_COPY(dst, src) do { \ + static size_t dst##__capa; \ I32 i; \ I32 top_index = av_len(src); \ I32 real_len = top_index + 1; \ I32 capa = real_len + 1; \ - dst = alloca(capa * sizeof(char *)); \ + if (capa > dst##__capa) { \ + dst##__capa = 0; /* in case Newx croaks */ \ + Safefree(dst); \ + Newx(dst, capa, char *); \ + dst##__capa = capa; \ + } \ for (i = 0; i < real_len; i++) { \ SV **sv = av_fetch(src, i, 0); \ dst[i] = SvPV_nolen(*sv); \ @@ -90,7 +88,7 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref, AV *rlim = (AV *)SvRV(rlimref); const char *filename = SvPV_nolen(file); pid_t pid = -1; - char **argv, **envp; + static char **argv, **envp; sigset_t set, old; int ret, perrnum; volatile int cerrnum = 0; /* shared due to vfork */ @@ -172,7 +170,8 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref, errno = perrnum; } out: - if (pid < 0) croak("E: fork_exec %s: %s\n", filename, strerror(errno)); + if (pid < 0) + croak("E: fork_exec %s: %s\n", filename, strerror(errno)); return (int)pid; } -- cgit v1.2.3-24-ge0c7