Date: Thu, 1 Dec 2011 00:16:46 +0000 From: Alexander Best <arundel@freebsd.org> To: Max Khon <fjoe@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r228157 - head/usr.bin/make Message-ID: <20111201001646.GA49249@freebsd.org> In-Reply-To: <201111301807.pAUI7cXI008371@svn.freebsd.org> References: <201111301807.pAUI7cXI008371@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed Nov 30 11, Max Khon wrote: > Author: fjoe > Date: Wed Nov 30 18:07:38 2011 > New Revision: 228157 > URL: http://svn.freebsd.org/changeset/base/228157 > > Log: > - Fix segmentation fault when running "+command" when run with -jX -n due > to Compat_RunCommand() being called with `cmd' that is not on the node->commands > list > - Make ellipsis ("..." command) handling consistent: check for "..." command > in job make after variables expansion to match compat make behavior > - Fix empty command handling (after variables expansion and @+- modifiers > are processed): now empty commands are ignored in compat make and are not > printed in job make case > - Bump MAKE_VERSION to 5-2011-11-30-0 it would also be nice, if at some point, somebody could dive into the code to see why 'make buildkernel' will let clang produce coloured output, but 'make -j(N>1) buildkernel' doesn't (and why adding a -B switch to that command fixes it). cheers. alex > > Modified: > head/usr.bin/make/Makefile > head/usr.bin/make/job.c > > Modified: head/usr.bin/make/Makefile > ============================================================================== > --- head/usr.bin/make/Makefile Wed Nov 30 17:39:00 2011 (r228156) > +++ head/usr.bin/make/Makefile Wed Nov 30 18:07:38 2011 (r228157) > @@ -10,7 +10,9 @@ SRCS= arch.c buf.c cond.c dir.c for.c ha > > NO_SHARED?= YES > > -CFLAGS+=-DMAKE_VERSION=\"5200408120\" > +# Version has the RYYYYMMDDX format, where R is from RELENG_<R> > +CFLAGS+=-DMAKE_VERSION=\"5201111300\" > + > # There is no obvious performance improvement currently. > # CFLAGS+=-DUSE_KQUEUE > > > Modified: head/usr.bin/make/job.c > ============================================================================== > --- head/usr.bin/make/job.c Wed Nov 30 17:39:00 2011 (r228156) > +++ head/usr.bin/make/job.c Wed Nov 30 18:07:38 2011 (r228157) > @@ -381,7 +381,7 @@ static int JobStart(GNode *, int, Job *) > static void JobDoOutput(Job *, Boolean); > static void JobInterrupt(int, int); > static void JobRestartJobs(void); > -static int Compat_RunCommand(char *, struct GNode *); > +static int Compat_RunCommand(LstNode *, struct GNode *); > > static GNode *curTarg = NULL; > static GNode *ENDNode; > @@ -647,7 +647,7 @@ JobPassSig(int signo) > * numCommands is incremented if the command is actually printed. > */ > static int > -JobPrintCommand(char *cmd, Job *job) > +JobPrintCommand(LstNode *cmdNode, Job *job) > { > Boolean noSpecials; /* true if we shouldn't worry about > * inserting special commands into > @@ -658,40 +658,30 @@ JobPrintCommand(char *cmd, Job *job) > * off before printing the command > * and need to turn it back on */ > const char *cmdTemplate;/* Template to use when printing the command */ > - char *cmdStart; /* Start of expanded command */ > - LstNode *cmdNode; /* Node for replacing the command */ > + char *cmd; /* Expanded command */ > > noSpecials = (noExecute && !(job->node->type & OP_MAKE)); > > - if (strcmp(cmd, "...") == 0) { > - job->node->type |= OP_SAVE_CMDS; > - if ((job->flags & JOB_IGNDOTS) == 0) { > - job->tailCmds = > - Lst_Succ(Lst_Member(&job->node->commands, cmd)); > - return (1); > - } > - return (0); > - } > - > #define DBPRINTF(fmt, arg) \ > DEBUGF(JOB, (fmt, arg)); \ > fprintf(job->cmdFILE, fmt, arg); \ > fflush(job->cmdFILE); > > - numCommands += 1; > - > /* > * For debugging, we replace each command with the result of expanding > * the variables in the command. > */ > - cmdNode = Lst_Member(&job->node->commands, cmd); > - > - cmd = Buf_Peel(Var_Subst(cmd, job->node, FALSE)); > - cmdStart = cmd; > - > - Lst_Replace(cmdNode, cmdStart); > - > - cmdTemplate = "%s\n"; > + cmd = Buf_Peel(Var_Subst(Lst_Datum(cmdNode), job->node, FALSE)); > + if (strcmp(cmd, "...") == 0) { > + free(cmd); > + job->node->type |= OP_SAVE_CMDS; > + if ((job->flags & JOB_IGNDOTS) == 0) { > + job->tailCmds = Lst_Succ(cmdNode); > + return (1); > + } > + return (0); > + } > + Lst_Replace(cmdNode, cmd); > > /* > * Check for leading @', -' or +'s to control echoing, error checking, > @@ -715,7 +705,7 @@ JobPrintCommand(char *cmd, Job *job) > * but this one needs to be - use compat mode > * just for it. > */ > - Compat_RunCommand(cmd, job->node); > + Compat_RunCommand(cmdNode, job->node); > return (0); > } > break; > @@ -726,6 +716,16 @@ JobPrintCommand(char *cmd, Job *job) > while (isspace((unsigned char)*cmd)) > cmd++; > > + /* > + * Ignore empty commands > + */ > + if (*cmd == '\0') { > + return (0); > + } > + > + cmdTemplate = "%s\n"; > + numCommands += 1; > + > if (shutUp) { > if (!(job->flags & JOB_SILENT) && !noSpecials && > commandShell->hasEchoCtl) { > @@ -1665,7 +1665,7 @@ JobStart(GNode *gn, int flags, Job *prev > Lst_Succ(gn->compat_command); > > if (gn->compat_command == NULL || > - JobPrintCommand(Lst_Datum(gn->compat_command), job)) > + JobPrintCommand(gn->compat_command, job)) > noExec = TRUE; > > if (noExec && !(job->flags & JOB_FIRST)) { > @@ -1689,7 +1689,7 @@ JobStart(GNode *gn, int flags, Job *prev > */ > numCommands = 0; > LST_FOREACH(ln, &gn->commands) { > - if (JobPrintCommand(Lst_Datum(ln), job)) > + if (JobPrintCommand(ln, job)) > break; > } > > @@ -1723,7 +1723,7 @@ JobStart(GNode *gn, int flags, Job *prev > */ > if (cmdsOK) { > LST_FOREACH(ln, &gn->commands) { > - if (JobPrintCommand(Lst_Datum(ln), job)) > + if (JobPrintCommand(ln, job)) > break; > } > } > @@ -2809,7 +2809,7 @@ CompatInterrupt(int signo) > gn = Targ_FindNode(".INTERRUPT", TARG_NOCREATE); > if (gn != NULL) { > LST_FOREACH(ln, &gn->commands) { > - if (Compat_RunCommand(Lst_Datum(ln), gn)) > + if (Compat_RunCommand(ln, gn)) > break; > } > } > @@ -2884,16 +2884,15 @@ shellneed(ArgArray *aa, char *cmd) > * The node's 'made' field may be set to ERROR. > */ > static int > -Compat_RunCommand(char *cmd, GNode *gn) > +Compat_RunCommand(LstNode *cmdNode, GNode *gn) > { > ArgArray aa; > - char *cmdStart; /* Start of expanded command */ > + char *cmd; /* Expanded command */ > Boolean silent; /* Don't print command */ > Boolean doit; /* Execute even in -n */ > Boolean errCheck; /* Check errors */ > int reason; /* Reason for child's death */ > int status; /* Description of child's death */ > - LstNode *cmdNode; /* Node where current cmd is located */ > char **av; /* Argument vector for thing to exec */ > ProcStuff ps; > > @@ -2901,31 +2900,16 @@ Compat_RunCommand(char *cmd, GNode *gn) > errCheck = !(gn->type & OP_IGNORE); > doit = FALSE; > > - cmdNode = Lst_Member(&gn->commands, cmd); > - cmdStart = Buf_Peel(Var_Subst(cmd, gn, FALSE)); > - > - /* > - * brk_string will return an argv with a NULL in av[0], thus causing > - * execvp() to choke and die horribly. Besides, how can we execute a > - * null command? In any case, we warn the user that the command > - * expanded to nothing (is this the right thing to do?). > - */ > - if (*cmdStart == '\0') { > - free(cmdStart); > - Error("%s expands to empty string", cmd); > - return (0); > - } else { > - cmd = cmdStart; > - } > - Lst_Replace(cmdNode, cmdStart); > - > + cmd = Buf_Peel(Var_Subst(Lst_Datum(cmdNode), gn, FALSE)); > if ((gn->type & OP_SAVE_CMDS) && (gn != ENDNode)) { > - Lst_AtEnd(&ENDNode->commands, cmdStart); > + Lst_AtEnd(&ENDNode->commands, cmd); > return (0); > - } else if (strcmp(cmdStart, "...") == 0) { > + } else if (strcmp(cmd, "...") == 0) { > + free(cmd); > gn->type |= OP_SAVE_CMDS; > return (0); > } > + Lst_Replace(cmdNode, cmd); > > while (*cmd == '@' || *cmd == '-' || *cmd == '+') { > switch (*cmd) { > @@ -2949,6 +2933,13 @@ Compat_RunCommand(char *cmd, GNode *gn) > cmd++; > > /* > + * Ignore empty commands > + */ > + if (*cmd == '\0') { > + return (0); > + } > + > + /* > * Print the command before echoing if we're not supposed to be quiet > * for this one. We also print the command if -n given, but not if '+'. > */ > @@ -3022,7 +3013,8 @@ Compat_RunCommand(char *cmd, GNode *gn) > * therefore do not free it when debugging. > */ > if (!DEBUG(GRAPH2)) { > - free(cmdStart); > + free(Lst_Datum(cmdNode)); > + Lst_Replace(cmdNode, NULL); > } > > /* > @@ -3166,8 +3158,7 @@ Compat_Make(GNode *gn, GNode *pgn) > if (!touchFlag) { > curTarg = gn; > LST_FOREACH(ln, &gn->commands) { > - if (Compat_RunCommand(Lst_Datum(ln), > - gn)) > + if (Compat_RunCommand(ln, gn)) > break; > } > curTarg = NULL; > @@ -3345,7 +3336,7 @@ Compat_Run(Lst *targs) > gn = Targ_FindNode(".BEGIN", TARG_NOCREATE); > if (gn != NULL) { > LST_FOREACH(ln, &gn->commands) { > - if (Compat_RunCommand(Lst_Datum(ln), gn)) > + if (Compat_RunCommand(ln, gn)) > break; > } > if (gn->made == ERROR) { > @@ -3386,7 +3377,7 @@ Compat_Run(Lst *targs) > */ > if (makeErrors == 0) { > LST_FOREACH(ln, &ENDNode->commands) { > - if (Compat_RunCommand(Lst_Datum(ln), ENDNode)) > + if (Compat_RunCommand(ln, ENDNode)) > break; > } > }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111201001646.GA49249>