On Mon, Aug 28, 2023 at 08:42:50PM +0200, Marc Espie wrote:
> Turns out the exec in cmd_exec.c has absolutely zero reason to be
> different from what engine does.
>
> This small patch moves a bit of code around, so that all execv() consumers
> benefit from the same optimisation (namely, no extra shell when not needed).
>
> The only visible change should be that now, VAR!=cmd *will* display
> some relevant information if exec fails.
>
> This is a fairly trivial change, I don't expect any fallout.
>
> (still need to run it through tests)
Better with the patch (thx miod@)
Index: cmd_exec.c
===================================================================
RCS file: /cvs/src/usr.bin/make/cmd_exec.c,v
retrieving revision 1.11
diff -u -p -r1.11 cmd_exec.c
--- cmd_exec.c 16 Jan 2020 16:07:18 -0000 1.11
+++ cmd_exec.c 28 Aug 2023 18:39:37 -0000
@@ -28,6 +28,8 @@
#include <sys/wait.h>
#include <errno.h>
#include <stdio.h>
+#include <stdbool.h>
+#include <string.h>
#include <unistd.h>
#include "config.h"
#include "defines.h"
@@ -36,11 +38,94 @@
#include "memory.h"
#include "pathnames.h"
#include "job.h"
+#include "str.h"
+
+/* The following array is used to make a fast determination of which
+ * characters are interpreted specially by the shell. If a command
+ * contains any of these characters, it is executed by the shell, not
+ * directly by us. */
+static char meta[256];
+static void retry_with_temp_file(bool, char **);
+
+void
+CmdExec_Init(void)
+{
+ char *p;
+
+ for (p = "#=|^(){};&<>*?[]:$`\\\n~"; *p != '\0'; p++)
+ meta[(unsigned char) *p] = 1;
+ /* The null character serves as a sentinel in the string. */
+ meta[0] = 1;
+}
+
+static char **
+recheck_command_for_shell(char **av)
+{
+ char *runsh[] = {
+ "!", "alias", "cd", "eval", "exit", "read", "set", "ulimit",
+ "unalias", "unset", "wait", "umask", NULL
+ };
+
+ char **p;
+
+ /* optimization: if exec cmd, we avoid the intermediate shell */
+ if (strcmp(av[0], "exec") == 0)
+ av++;
+
+ if (!av[0])
+ return NULL;
+
+ for (p = runsh; *p; p++)
+ if (strcmp(av[0], *p) == 0)
+ return NULL;
+
+ return av;
+}
+
+void
+run_command(const char *cmd, bool errCheck)
+{
+ const char *p;
+ char *shargv[4];
+ char **todo;
+
+ shargv[0] = _PATH_BSHELL;
+
+ shargv[1] = errCheck ? "-ec" : "-c";
+ shargv[2] = (char *)cmd;
+ shargv[3] = NULL;
+
+ todo = shargv;
+
+
+ /* Search for meta characters in the command. If there are no meta
+ * characters, there's no need to execute a shell to execute the
+ * command. */
+ for (p = cmd; !meta[(unsigned char)*p]; p++)
+ continue;
+ if (*p == '\0') {
+ char *bp;
+ char **av;
+ int argc;
+ /* No meta-characters, so probably no need to exec a shell.
+ * Break the command into words to form an argument vector
+ * we can execute. */
+ av = brk_string(cmd, &argc, &bp);
+ av = recheck_command_for_shell(av);
+ if (av != NULL)
+ todo = av;
+ }
+ execvp(todo[0], todo);
+ if (errno == ENOENT)
+ fprintf(stderr, "%s: not found\n", todo[0]);
+ else
+ perror(todo[0]);
+ _exit(1);
+}
char *
Cmd_Exec(const char *cmd, char **err)
{
- char *args[4]; /* Args for invoking the shell */
int fds[2]; /* Pipe streams */
pid_t cpid; /* Child PID */
char *result; /* Result */
@@ -53,12 +138,6 @@ Cmd_Exec(const char *cmd, char **err)
*err = NULL;
- /* Set up arguments for the shell. */
- args[0] = "sh";
- args[1] = "-c";
- args[2] = (char *)cmd;
- args[3] = NULL;
-
/* Open a pipe for retrieving shell's output. */
if (pipe(fds) == -1) {
*err = "Couldn't create pipe for \"%s\"";
@@ -82,8 +161,7 @@ Cmd_Exec(const char *cmd, char **err)
(void)close(fds[1]);
}
- (void)execv(_PATH_BSHELL, args);
- _exit(1);
+ run_command(cmd, false);
/*NOTREACHED*/
case -1:
Index: cmd_exec.h
===================================================================
RCS file: /cvs/src/usr.bin/make/cmd_exec.h,v
retrieving revision 1.4
diff -u -p -r1.4 cmd_exec.h
--- cmd_exec.h 19 Jul 2010 19:46:43 -0000 1.4
+++ cmd_exec.h 28 Aug 2023 18:39:37 -0000
@@ -34,4 +34,6 @@
* The output result should always be freed by the caller. */
extern char *Cmd_Exec(const char *, char **);
+extern void CmdExec_Init(void);
+extern __dead void run_command(const char *, bool);
#endif
Index: engine.c
===================================================================
RCS file: /cvs/src/usr.bin/make/engine.c,v
retrieving revision 1.71
diff -u -p -r1.71 engine.c
--- engine.c 30 May 2023 04:42:21 -0000 1.71
+++ engine.c 28 Aug 2023 18:39:37 -0000
@@ -70,10 +70,12 @@
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
+#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include "config.h"
+#include "cmd_exec.h"
#include "defines.h"
#include "dir.h"
#include "engine.h"
@@ -88,7 +90,6 @@
#include "make.h"
#include "pathnames.h"
#include "error.h"
-#include "str.h"
#include "memory.h"
#include "buf.h"
#include "job.h"
@@ -96,9 +97,6 @@
static void MakeTimeStamp(void *, void *);
static int rewrite_time(const char *);
-static void setup_meta(void);
-static void setup_engine(void);
-static char **recheck_command_for_shell(char **);
static void list_parents(GNode *, FILE *);
/* XXX due to a bug in make's logic, targets looking like *.a or -l*
@@ -508,88 +506,6 @@ Make_OODate(GNode *gn)
return oodate;
}
-/* The following array is used to make a fast determination of which
- * characters are interpreted specially by the shell. If a command
- * contains any of these characters, it is executed by the shell, not
- * directly by us. */
-static char meta[256];
-
-void
-setup_meta(void)
-{
- char *p;
-
- for (p = "#=|^(){};&<>*?[]:$`\\\n~"; *p != '\0'; p++)
- meta[(unsigned char) *p] = 1;
- /* The null character serves as a sentinel in the string. */
- meta[0] = 1;
-}
-
-static char **
-recheck_command_for_shell(char **av)
-{
- char *runsh[] = {
- "!", "alias", "cd", "eval", "exit", "read", "set", "ulimit",
- "unalias", "unset", "wait", "umask", NULL
- };
-
- char **p;
-
- /* optimization: if exec cmd, we avoid the intermediate shell */
- if (strcmp(av[0], "exec") == 0)
- av++;
-
- if (!av[0])
- return NULL;
-
- for (p = runsh; *p; p++)
- if (strcmp(av[0], *p) == 0)
- return NULL;
-
- return av;
-}
-
-static void
-run_command(const char *cmd, bool errCheck)
-{
- const char *p;
- char *shargv[4];
- char **todo;
-
- shargv[0] = _PATH_BSHELL;
-
- shargv[1] = errCheck ? "-ec" : "-c";
- shargv[2] = (char *)cmd;
- shargv[3] = NULL;
-
- todo = shargv;
-
-
- /* Search for meta characters in the command. If there are no meta
- * characters, there's no need to execute a shell to execute the
- * command. */
- for (p = cmd; !meta[(unsigned char)*p]; p++)
- continue;
- if (*p == '\0') {
- char *bp;
- char **av;
- int argc;
- /* No meta-characters, so probably no need to exec a shell.
- * Break the command into words to form an argument vector
- * we can execute. */
- av = brk_string(cmd, &argc, &bp);
- av = recheck_command_for_shell(av);
- if (av != NULL)
- todo = av;
- }
- execvp(todo[0], todo);
-
- if (errno == ENOENT)
- fprintf(stderr, "%s: not found\n", todo[0]);
- else
- perror(todo[0]);
- _exit(1);
-}
void
job_attach_node(Job *job, GNode *node)
@@ -696,17 +612,6 @@ run_gnode(GNode *gn)
}
-static void
-setup_engine(void)
-{
- static int already_setup = 0;
-
- if (!already_setup) {
- setup_meta();
- already_setup = 1;
- }
-}
-
static bool
do_run_command(Job *job, const char *pre)
{
@@ -799,7 +704,6 @@ job_run_next(Job *job)
bool started;
GNode *gn = job->node;
- setup_engine();
while (job->next_cmd != NULL) {
struct command *command = Lst_Datum(job->next_cmd);
Index: init.c
===================================================================
RCS file: /cvs/src/usr.bin/make/init.c,v
retrieving revision 1.8
diff -u -p -r1.8 init.c
--- init.c 16 Jan 2020 16:07:18 -0000 1.8
+++ init.c 28 Aug 2023 18:39:37 -0000
@@ -37,11 +37,13 @@
#include "targ.h"
#include "suff.h"
#include "job.h"
+#include "cmd_exec.h"
void
Init(void)
{
Sigset_Init();
+ CmdExec_Init();
Init_Timestamp();
Init_Stats();
Targ_Init();
