From 21d15001f9d130fca97c368d713413d573aeedb9 Mon Sep 17 00:00:00 2001 From: Hartmut Brandt Date: Thu, 25 Nov 2004 10:01:26 +0000 Subject: [PATCH] Fix a very long-standing error in handling .SHELL targets: this target uses the brk_string function to parse the line. That function uses static storage for both the expanded string and the returned argv[] vector. The JobParseShell function simply stored away pointers into this static storage. On the next use of something like ${FOO:O} this storage would get overwritten with fatal results. This also allows us to make the shells[] array const bringing us one step further in making make WARNS=4 ready. --- usr.bin/make/job.c | 177 ++++++++++++++++++++++++++++++++------------- 1 file changed, 128 insertions(+), 49 deletions(-) diff --git a/usr.bin/make/job.c b/usr.bin/make/job.c index 0b86bb0c3d2c..50571e4d4a32 100644 --- a/usr.bin/make/job.c +++ b/usr.bin/make/job.c @@ -206,14 +206,11 @@ static Shell shells[] = { "v", "e", }, }; -static Shell *commandShell = &shells[DEFSHELL];/* this is the shell to - * which we pass all - * commands in the Makefile. - * It is set by the - * Job_ParseShell function */ -char *shellPath = NULL, /* full pathname of - * executable image */ - *shellName = NULL; /* last component of shell */ +static Shell *commandShell = NULL; /* this is the shell to which we pass + * all commands in the Makefile. It is + * set by the Job_ParseShell function */ +char *shellPath = NULL, /* full pathname of executable image */ + *shellName = NULL; /* last component of shell */ static int maxJobs; /* The most children we can run at once */ @@ -2071,9 +2068,90 @@ Job_Make(GNode *gn) (void) JobStart(gn, 0, NULL); } +/* + * JobCopyShell: + * + * Make a new copy of the shell structure including a copy of the strings + * in it. This also defaults some fields in case they are NULL. + * + * The function returns a pointer to the new shell structure otherwise. + */ +static Shell * +JobCopyShell(const Shell *osh) +{ + Shell *nsh; + + nsh = emalloc(sizeof(*nsh)); + nsh->name = estrdup(osh->name); + + if (osh->echoOff != NULL) + nsh->echoOff = estrdup(osh->echoOff); + else + nsh->echoOff = NULL; + if (osh->echoOn != NULL) + nsh->echoOn = estrdup(osh->echoOn); + else + nsh->echoOn = NULL; + nsh->hasEchoCtl = osh->hasEchoCtl; + + if (osh->noPrint != NULL) + nsh->noPrint = estrdup(osh->noPrint); + else + nsh->noPrint = NULL; + nsh->noPLen = osh->noPLen; + + nsh->hasErrCtl = osh->hasErrCtl; + if (osh->errCheck == NULL) + nsh->errCheck = estrdup(""); + else + nsh->errCheck = estrdup(osh->errCheck); + if (osh->ignErr == NULL) + nsh->ignErr = estrdup("%s"); + else + nsh->ignErr = estrdup(osh->ignErr); + + if (osh->echo == NULL) + nsh->echo = estrdup(""); + else + nsh->echo = estrdup(osh->echo); + + if (osh->exit == NULL) + nsh->exit = estrdup(""); + else + nsh->exit = estrdup(osh->exit); + + return (nsh); +} + +/* + * JobFreeShell: + * + * Free a shell structure and all associated strings. + */ +static void +JobFreeShell(Shell *sh) +{ + + if (sh != NULL) { + free(sh->name); + free(sh->echoOff); + free(sh->echoOn); + free(sh->noPrint); + free(sh->errCheck); + free(sh->ignErr); + free(sh->echo); + free(sh->exit); + free(sh); + } +} + void Shell_Init(void) { + + if (commandShell == NULL) + commandShell = JobCopyShell(&shells[DEFSHELL]); + if (shellPath == NULL) { /* * The user didn't specify a shell to use, so we are using the @@ -2451,6 +2529,18 @@ Job_ParseShell(char *line) } } + /* + * Some checks (could be more) + */ + if (fullSpec) { + if ((newShell.echoOn != NULL) ^ (newShell.echoOff != NULL)) + Parse_Error(PARSE_FATAL, "Shell must have either both echoOff and " + "echoOn or none of them"); + + if (newShell.echoOn != NULL && newShell.echoOff) + newShell.hasEchoCtl = TRUE; + } + if (path == NULL) { /* * If no path was given, the user wants one of the pre-defined shells, @@ -2460,16 +2550,13 @@ Job_ParseShell(char *line) */ if (newShell.name == NULL) { Parse_Error(PARSE_FATAL, "Neither path nor name specified"); - return(FAILURE); - } else { - if ((sh = JobMatchShell(newShell.name)) == NULL) { - Parse_Error(PARSE_FATAL, "%s: no matching shell", - newShell.name); - return(FAILURE); - } - commandShell = sh; - shellName = newShell.name; + return (FAILURE); } + if ((sh = JobMatchShell(newShell.name)) == NULL) { + Parse_Error(PARSE_FATAL, "%s: no matching shell", newShell.name); + return (FAILURE); + } + } else { /* * The user provided a path. If s/he gave nothing else (fullSpec is @@ -2478,45 +2565,37 @@ Job_ParseShell(char *line) * to a new location. In either case, we need to record the * path the user gave for the shell. */ - shellPath = path; - path = strrchr(path, '/'); - if (path == NULL) { - path = shellPath; - } else { - path += 1; - } - if (newShell.name != NULL) { - shellName = newShell.name; - } else { - shellName = path; - } - if (!fullSpec) { - if ((sh = JobMatchShell(shellName)) == NULL) { - Parse_Error(PARSE_FATAL, "%s: no matching shell", - shellName); - return(FAILURE); + free(shellPath); + shellPath = estrdup(path); + if (newShell.name == NULL) { + /* get the base name as the name */ + path = strrchr(path, '/'); + if (path == NULL) { + path = shellPath; + } else { + path += 1; + } + newShell.name = path; + } + + if (!fullSpec) { + if ((sh = JobMatchShell(newShell.name)) == NULL) { + Parse_Error(PARSE_FATAL, "%s: no matching shell", + newShell.name); + return (FAILURE); } - commandShell = sh; } else { - commandShell = (Shell *) emalloc(sizeof(Shell)); - *commandShell = newShell; + sh = &newShell; } } - if (commandShell->echoOn && commandShell->echoOff) { - commandShell->hasEchoCtl = TRUE; - } + /* set the new shell */ + JobFreeShell(commandShell); + commandShell = JobCopyShell(sh); - if (!commandShell->hasErrCtl) { - if (commandShell->errCheck == NULL) { - commandShell->errCheck = ""; - } - if (commandShell->ignErr == NULL) { - commandShell->ignErr = "%s\n"; - } - } + shellName = commandShell->name; - return SUCCESS; + return (SUCCESS); } /*-