From owner-freebsd-bugs@FreeBSD.ORG Sun Jun 10 12:00:17 2007 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 123B016A4CD for ; Sun, 10 Jun 2007 12:00:17 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [69.147.83.40]) by mx1.freebsd.org (Postfix) with ESMTP id EB0AB13C45A for ; Sun, 10 Jun 2007 12:00:16 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id l5AC0GfP006655 for ; Sun, 10 Jun 2007 12:00:16 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id l5AC0G48006591; Sun, 10 Jun 2007 12:00:16 GMT (envelope-from gnats) Resent-Date: Sun, 10 Jun 2007 12:00:16 GMT Resent-Message-Id: <200706101200.l5AC0G48006591@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Ed Schouten Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C26C816A400 for ; Sun, 10 Jun 2007 11:52:46 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from palm.hoeg.nl (mx0.hoeg.nl [83.98.131.211]) by mx1.freebsd.org (Postfix) with ESMTP id 71C8F13C458 for ; Sun, 10 Jun 2007 11:52:46 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: by palm.hoeg.nl (Postfix, from userid 1000) id 15D871CC5A; Sun, 10 Jun 2007 13:52:45 +0200 (CEST) Message-Id: <20070610115245.15D871CC5A@palm.hoeg.nl> Date: Sun, 10 Jun 2007 13:52:45 +0200 (CEST) From: Ed Schouten To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: bin/113518: make(1): Prevent execution when command is a comment X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Ed Schouten List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jun 2007 12:00:17 -0000 >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@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: