bin/113518: make(1): Prevent execution when command is a comment

Ed Schouten ed at fxq.nl
Sun Jun 10 12:00:17 UTC 2007


>Number:         113518
>Category:       bin
>Synopsis:       make(1): Prevent execution when command is a comment
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Jun 10 12:00:11 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Ed Schouten
>Release:        FreeBSD 6.2-STABLE i386
>Organization:
>Environment:
System: FreeBSD palm.hoeg.nl 6.2-STABLE FreeBSD 6.2-STABLE #0: Fri Apr 20 13:44:49 CEST 2007 root at palm.hoeg.nl:/usr/obj/usr/src/sys/PALM i386
>Description:
The make(1) source code contains a lot of optimizations to make sure it
calls applications like shells as less as possible, so commands without
special characters in them are just splitted by whitespace and executed.

For some reason, make(1) doesn't have the knowledge that it can just
skip lines starting with '#' when using sh(1), csh(1) or ksh(1), because
they won't have any effect at all.
>How-To-Repeat:
Generate a Makefile like this:

| (echo example:
| for i in `jot 10000`
| do
|         printf '\t# %d\n' $i
| done) > Makefile

When using the stock FreeBSD make(1) or gmake(1), it takes a lot of time
to execute this command:

| $ time make > /dev/null
| make > /dev/null  14.91s user 18.58s system 95% cpu 34.959 total
| $ time gmake > /dev/null
| gmake > /dev/null  13.05s user 19.00s system 98% cpu 32.681 total

When we teach make(1) to just ignore lines starting with '#', our
results are a lot better:

| $ time make > /dev/null 
| make > /dev/null  0.26s user 0.01s system 99% cpu 0.269 total

>Fix:
The following patch adds a new parameter to shell.c called `skipfirst'
which holds a list of characters that mean we can skip this command if
it's the first character in the string. This patch shouldn't break any
exotic makefiles, because it only performs this optimization when sh(1),
csh(1) or ksh(1) is used.

Index: job.c
===================================================================
RCS file: /datadump/cvs/freebsd/src/usr.bin/make/job.c,v
retrieving revision 1.126
diff -u -r1.126 job.c
--- job.c	8 Mar 2007 09:16:10 -0000	1.126
+++ job.c	10 Jun 2007 11:39:45 -0000
@@ -2817,6 +2817,24 @@
 }
 
 /**
+ * shellskip
+ *
+ * Results:
+ *      Returns a non-zero value if execution of the command wouldn't
+ *      have an effect at all, which allows us to skip this line. This
+ *      is useful for lines starting with '#'.
+ */
+static int
+shellskip(char *cmd)
+{
+	if (commandShell->skipfirst != NULL &&
+	    strchr(commandShell->skipfirst, cmd[0]) != NULL)
+		return (1);
+
+	return (0);
+}
+
+/**
  * shellneed
  *
  * Results:
@@ -2958,6 +2976,13 @@
 		return (0);
 	}
 
+	/*
+	 * Don't execute the command if it's just a comment.
+	 */
+	if (shellskip(cmd)) {
+		return (0);
+	}
+
 	ps.in = STDIN_FILENO;
 	ps.out = STDOUT_FILENO;
 	ps.err = STDERR_FILENO;
Index: shell.c
===================================================================
RCS file: /datadump/cvs/freebsd/src/usr.bin/make/shell.c,v
retrieving revision 1.1
diff -u -r1.1 shell.c
--- shell.c	24 May 2005 15:30:03 -0000	1.1
+++ shell.c	10 Jun 2007 11:51:31 -0000
@@ -82,7 +82,7 @@
 	"name=csh path='" PATH_DEFSHELLDIR "/csh' "
 	"quiet='unset verbose' echo='set verbose' filter='unset verbose' "
 	"hasErrCtl=N check='echo \"%s\"\n' ignore='csh -c \"%s || exit 0\"' "
-	"echoFlag=v errFlag=e "
+	"echoFlag=v errFlag=e skipfirst=# "
 	"meta='" CSH_META "' builtins='" CSH_BUILTINS "'",
 
 	/*
@@ -92,7 +92,7 @@
 	"name=sh path='" PATH_DEFSHELLDIR "/sh' "
 	"quiet='set -' echo='set -v' filter='set -' "
 	"hasErrCtl=Y check='set -e' ignore='set +e' "
-	"echoFlag=v errFlag=e "
+	"echoFlag=v errFlag=e skipfirst=# "
 	"meta='" SH_META "' builtins='" SH_BUILTINS "'",
 
 	/*
@@ -103,7 +103,7 @@
 	"name=ksh path='" PATH_DEFSHELLDIR "/ksh' "
 	"quiet='set -' echo='set -v' filter='set -' "
 	"hasErrCtl=Y check='set -e' ignore='set +e' "
-	"echoFlag=v errFlag=e "
+	"echoFlag=v errFlag=e skipfirst=# "
 	"meta='" SH_META "' builtins='" SH_BUILTINS "' unsetenv=T",
 
 	NULL
@@ -150,6 +150,7 @@
 		free(sh->echo);
 		free(sh->exit);
 		ArgArray_Done(&sh->builtins);
+		free(sh->skipfirst);
 		free(sh->meta);
 		free(sh);
 	}
@@ -174,7 +175,8 @@
 	fprintf(stderr, "  builtins=%d\n", sh->builtins.argc - 1);
 	for (i = 1; i < sh->builtins.argc; i++)
 		fprintf(stderr, " '%s'", sh->builtins.argv[i]);
-	fprintf(stderr, "\n  meta='%s'\n", sh->meta);
+	fprintf(stderr, "\n  skipfirst='%s'\n", sh->skipfirst);
+	fprintf(stderr, "  meta='%s'\n", sh->meta);
 	fprintf(stderr, "  unsetenv=%d\n", sh->unsetenv);
 }
 
@@ -260,6 +262,10 @@
 			qsort(sh->builtins.argv + 1, sh->builtins.argc - 1,
 			    sizeof(char *), sort_builtins);
 			*fullSpec = TRUE;
+		} else if (strcmp(keyw, "skipfirst") == 0) {
+			free(sh->skipfirst);
+			sh->skipfirst = estrdup(eq);
+			*fullSpec = TRUE;
 		} else if (strcmp(keyw, "meta") == 0) {
 			free(sh->meta);
 			sh->meta = estrdup(eq);
@@ -379,6 +385,8 @@
  *			    execute the command directly. If this list is empty
  *			    it is assumed, that the command must always be
  *			    handed over to the shell.
+ *	    skipfirst	    Skip execution of the command if the first
+ *	    		    character has a certain value.
  *	    meta	    The shell meta characters. If this is not specified
  *			    or empty, commands are alway passed to the shell.
  *			    Otherwise they are not passed when they contain
Index: shell.h
===================================================================
RCS file: /datadump/cvs/freebsd/src/usr.bin/make/shell.h,v
retrieving revision 1.1
diff -u -r1.1 shell.h
--- shell.h	24 May 2005 15:30:03 -0000	1.1
+++ shell.h	10 Jun 2007 11:39:45 -0000
@@ -96,6 +96,7 @@
 	char	*exit;	/* command line flag: exit on error */
 
 	ArgArray builtins;	/* ordered list of shell builtins */
+	char	*skipfirst;	/* characters that determine if we can skip the line */
 	char	*meta;		/* shell meta characters */
 
 	Boolean	unsetenv;	/* unsetenv("ENV") before exec */
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list