Get rid of global variables for argument vectors produced by brk_string()

introduce a struct that holds all the information about an argument
vector and pass that around.

Author:		Max Okumoto <okumoto@ucsd.edu>
Obtained from:	DragonFlyBSD

7 files changed, 263 insertions, 211 deletions

diff --git a/usr.bin/make/job.c b/usr.bin/make/job.c
index 96a5989..b49602c 100644
--- a/usr.bin/make/job.c
+++ b/usr.bin/make/job.c
@@ -2895,40 +2895,34 @@ JobMatchShell(const char *name)
  *			    hasErrCtl is FALSE.
  */
 Boolean
-Job_ParseShell(char *line)
+Job_ParseShell(const char line[])
 {
-	char	**words;
-	int	wordCount;
-	char	**argv;
-	int	argc;
-	char	*path;
-	char	*eq;
-	Boolean	fullSpec = FALSE;
+	ArgArray	aa;
+	char		**argv;
+	int		argc;
+	char		*path;
+	char		*eq;
+	Boolean		fullSpec = FALSE;
 	struct Shell	newShell;
 	struct Shell	*sh;
 
-	while (isspace((unsigned char)*line)) {
-		line++;
-	}
-
 	memset(&newShell, 0, sizeof(newShell));
 	path = NULL;
 
 	/*
-	 * Parse the specification by keyword but skip the first word - it
-	 * is not set by brk_string.
+	 * Parse the specification by keyword but skip the first word
 	 */
-	words = brk_string(line, &wordCount, TRUE);
-	words++;
-	wordCount--;
+	brk_string(&aa, line, TRUE);
 
-	for (argc = wordCount, argv = words; argc != 0; argc--, argv++) {
+	for (argc = aa.argc - 1, argv = aa.argv + 1; argc != 0;
+	    argc--, argv++) {
 		/*
 		 * Split keyword and value
 		 */
 		if ((eq = strchr(*argv, '=')) == NULL) {
 			Parse_Error(PARSE_FATAL, "missing '=' in shell "
 			    "specification keyword '%s'", *argv);
+			ArgArray_Done(&aa);
 			return (FALSE);
 		}
 		*eq++ = '\0';
@@ -2965,6 +2959,7 @@ Job_ParseShell(char *line)
 		} else {
 			Parse_Error(PARSE_FATAL, "unknown keyword in shell "
 			    "specification '%s'", *argv);
+			ArgArray_Done(&aa);
 			return (FALSE);
 		}
 	}
@@ -2991,11 +2986,13 @@ Job_ParseShell(char *line)
 		if (newShell.name == NULL) {
 			Parse_Error(PARSE_FATAL,
 			    "Neither path nor name specified");
+			ArgArray_Done(&aa);
 			return (FALSE);
 		}
 		if ((sh = JobMatchShell(newShell.name)) == NULL) {
 			Parse_Error(PARSE_FATAL, "%s: no matching shell",
 			    newShell.name);
+			ArgArray_Done(&aa);
 			return (FALSE);
 		}
 
@@ -3022,6 +3019,7 @@ Job_ParseShell(char *line)
 				Parse_Error(PARSE_FATAL,
 				    "%s: no matching shell", newShell.name);
 				free(path);
+				ArgArray_Done(&aa);
 				return (FALSE);
 			}
 		} else {
@@ -3037,6 +3035,7 @@ Job_ParseShell(char *line)
 
 	shellName = commandShell->name;
 
+	ArgArray_Done(&aa);
 	return (TRUE);
 }
 
@@ -3391,9 +3390,8 @@ CompatInterrupt(int signo)
  *	Uses brk_string so destroys the contents of argv.
  */
 static char **
-shellneed(char *cmd)
+shellneed(ArgArray *aa, char *cmd)
 {
-	char		**av;
 	const char	**p;
 
 	if (strpbrk(cmd, sh_meta) != NULL)
@@ -3401,14 +3399,16 @@ shellneed(char *cmd)
 
 	/*
 	 * Break the command into words to form an argument
-	 * vector we can execute. brk_string sticks NULL
-	 * in av[0], so we have to skip over it...
+	 * vector we can execute.
 	 */
-	av = brk_string(cmd, NULL, TRUE);
-	for (p = sh_builtin; *p != 0; p++)
-		if (strcmp(av[1], *p) == 0)
+	brk_string(aa, cmd, TRUE);
+	for (p = sh_builtin; *p != 0; p++) {
+		if (strcmp(aa->argv[1], *p) == 0) {
+			ArgArray_Done(aa);
 			return (NULL);
-	return (av + 1);
+		}
+	}
+	return (aa->argv + 1);
 }
 
 /*-
@@ -3429,14 +3429,15 @@ shellneed(char *cmd)
 static int
 Compat_RunCommand(char *cmd, GNode *gn)
 {
-	char	*cmdStart;	/* Start of 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 command is located */
-	char	**av;		/* Argument vector for thing to exec */
+	ArgArray	aa;
+	char		*cmdStart;	/* Start of 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;
 
 	silent = gn->type & OP_SILENT;
@@ -3515,7 +3516,7 @@ Compat_RunCommand(char *cmd, GNode *gn)
 	ps.pgroup = 0;
 	ps.searchpath = 1;
 
-	if ((av = shellneed(cmd)) == NULL) {
+	if ((av = shellneed(&aa, cmd)) == NULL) {
 		/*
 		 * Shell meta character or shell builtin found - pass
 		 * command to shell. We give the shell the -e flag as
@@ -3553,6 +3554,8 @@ Compat_RunCommand(char *cmd, GNode *gn)
 			free(ps.argv[1]);
 			free(ps.argv[0]);
 			free(ps.argv);
+		} else {
+			ArgArray_Done(&aa);
 		}
 
 		/*
diff --git a/usr.bin/make/job.h b/usr.bin/make/job.h
index c838c1c..45518de 100644
--- a/usr.bin/make/job.h
+++ b/usr.bin/make/job.h
@@ -64,7 +64,7 @@ void Job_Make(struct GNode *);
 void Job_Init(int);
 Boolean Job_Full(void);
 Boolean Job_Empty(void);
-Boolean Job_ParseShell(char *);
+Boolean Job_ParseShell(const char []);
 int Job_Finish(void);
 void Job_Wait(void);
 void Job_AbortAll(void);
diff --git a/usr.bin/make/main.c b/usr.bin/make/main.c
index ad20f32..1d65ffb 100644
--- a/usr.bin/make/main.c
+++ b/usr.bin/make/main.c
@@ -575,8 +575,7 @@ rearg:
 void
 Main_ParseArgLine(char *line, int mflags)
 {
-	char **argv;			/* Manufactured argument vector */
-	int argc;			/* Number of arguments in argv */
+	ArgArray	aa;
 
 	if (line == NULL)
 		return;
@@ -586,11 +585,12 @@ Main_ParseArgLine(char *line, int mflags)
 		return;
 
 	if (mflags)
-		argv = MAKEFLAGS_break(line, &argc);
+		MAKEFLAGS_break(&aa, line);
 	else
-		argv = brk_string(line, &argc, TRUE);
+		brk_string(&aa, line, TRUE);
 
-	MainParseArgs(argc, argv);
+	MainParseArgs(aa.argc, aa.argv);
+	ArgArray_Done(&aa);
 }
 
 static char *
@@ -785,8 +785,6 @@ main(int argc, char **argv)
 				 * can be processed correctly */
 	Var_Init(environ);	/* As well as the lists of variables for
 				 * parsing arguments */
-        str_init();
-
 	/*
 	 * Initialize various variables.
 	 *	MAKE also gets this name, for compatibility
diff --git a/usr.bin/make/parse.c b/usr.bin/make/parse.c
index ccccf23..2f0a425 100644
--- a/usr.bin/make/parse.c
+++ b/usr.bin/make/parse.c
@@ -405,14 +405,13 @@ ParsePopInput(void)
 static void
 parse_warn(char *line)
 {
-	char **argv;
-	int argc;
-	int i;
+	ArgArray	aa;
+	int		i;
 
-	argv = brk_string(line, &argc, TRUE);
+	brk_string(&aa, line, TRUE);
 
-	for (i = 1; i < argc; i++)
-		Main_ParseWarn(argv[i], 0);
+	for (i = 1; i < aa.argc; i++)
+		Main_ParseWarn(aa.argv[i], 0);
 }
 
 /*-
diff --git a/usr.bin/make/str.c b/usr.bin/make/str.c
index 372b3d8..4c508b8 100644
--- a/usr.bin/make/str.c
+++ b/usr.bin/make/str.c
@@ -50,23 +50,50 @@ __FBSDID("$FreeBSD$");
 #include "str.h"
 #include "util.h"
 
-static char **argv;
-static char *buffer;
-static int argmax;
-static int curlen;
+/**
+ * Initialize the argument array object.  The array is initially
+ * eight positions, and will be expaned as neccessary.  The first
+ * position is set to NULL since everything ignores it.  We allocate
+ * (size + 1) since we need space for the terminating NULL.  The
+ * buffer is set to NULL, since no common buffer is alloated yet.
+ */
+static void
+ArgArray_Init(ArgArray *aa)
+{
 
-/*
- * str_init --
- *	Initialize the strings package
- *
+	aa->size = 8;
+	aa->argv = emalloc((aa->size + 1) * sizeof(char *));
+	aa->argc = 0;
+	aa->argv[aa->argc++] = NULL;
+	aa->len = 0;
+	aa->buffer = NULL;
+}
+
+/**
+ * Cleanup the memory allocated for in the argument array object. 
  */
 void
-str_init(void)
+ArgArray_Done(ArgArray *aa)
 {
 
-	argmax = 50;
-	argv = emalloc((argmax + 1) * sizeof(char *));
-	argv[0] = NULL;
+	if (aa->buffer == NULL) {
+		int	i;
+		/* args are individually allocated */
+		for (i = 0; i < aa->argc; ++i) {
+			if (aa->argv[i]) {
+				free(aa->argv[i]);
+				aa->argv[i] = NULL;
+			}
+		}
+	} else {
+		/* args are part of a single allocation */
+		free(aa->buffer);
+		aa->buffer = NULL;
+	}
+	free(aa->argv);
+	aa->argv = NULL;
+	aa->argc = 0;
+	aa->size = 0;
 }
 
 /*-
@@ -107,130 +134,144 @@ str_concat(const char *s1, const char *s2, int flags)
 	return (result);
 }
 
-/*-
- * brk_string --
- *	Fracture a string into an array of words (as delineated by tabs or
- *	spaces) taking quotation marks into account.  Leading tabs/spaces
- *	are ignored.
- *
- * returns --
- *	Pointer to the array of pointers to the words.
+/**
+ * Fracture a string into an array of words (as delineated by tabs or
+ * spaces) taking quotation marks into account.  Leading tabs/spaces
+ * are ignored.
  */
-char **
-brk_string(const char *str, int *store_argc, Boolean expand)
+void
+brk_string(ArgArray *aa, const char str[], Boolean expand)
 {
-	int argc, ch;
-	char inquote;
-	const char *p;
-	char *start, *t;
-	int len;
+	char	inquote;
+	char	*start;
+	char	*arg;
 
 	/* skip leading space chars. */
 	for (; *str == ' ' || *str == '\t'; ++str)
 		continue;
 
-	/* allocate room for a copy of the string */
-	if ((len = strlen(str) + 1) > curlen) {
-		if (buffer)
-		    free(buffer);
-		buffer = emalloc(curlen = len);
-	}
+	ArgArray_Init(aa);
+
+	aa->buffer = estrdup(str);;
+
+	arg = aa->buffer;
+	start = arg;
+	inquote = '\0';
 
 	/*
 	 * copy the string; at the same time, parse backslashes,
 	 * quotes and build the argument list.
 	 */
-	argc = 1;
-	inquote = '\0';
-	for (p = str, start = t = buffer;; ++p) {
-		switch(ch = *p) {
+	for (;;) {
+		switch (str[0]) {
 		case '"':
 		case '\'':
-			if (inquote) {
-				if (ch != inquote)
+			if (inquote == '\0') {
+				inquote = str[0];
+				if (expand)
 					break;
+				if (start == NULL)
+					start = arg;
+			} else if (inquote == str[0]) {
 				inquote = '\0';
 				/* Don't miss "" or '' */
-				if (!start)
-					start = t;
-			} else
-				inquote = (char)ch;
-			if (expand)
-				continue;
+				if (start == NULL)
+					start = arg;
+				if (expand)
+					break;
+			} else {
+				/* other type of quote found */
+				if (start == NULL)
+					start = arg;
+			}
+			*arg++ = str[0];
 			break;
 		case ' ':
 		case '\t':
 		case '\n':
-			if (inquote)
+			if (inquote) {
+				if (start == NULL)
+					start = arg;
+				*arg++ = str[0];
+				break;
+			}
+			if (start == NULL)
 				break;
-			if (!start)
-				continue;
 			/* FALLTHROUGH */
 		case '\0':
 			/*
 			 * end of a token -- make sure there's enough argv
 			 * space and save off a pointer.
 			 */
-			if (!start)
-			    goto done;
-
-			*t++ = '\0';
-			if (argc == argmax) {
-				argmax *= 2;		/* ramp up fast */
-				argv = erealloc(argv,
-				    (argmax + 1) * sizeof(char *));
-			}
-			argv[argc++] = start;
-			start = NULL;
-			if (ch == '\n' || ch == '\0')
-				goto done;
-			continue;
-		case '\\':
-			if (!expand) {
-				if (!start)
-					start = t;
-				*t++ = '\\';
-				ch = *++p;
-				break;
+			if (aa->argc == aa->size) {
+				aa->size *= 2;		/* ramp up fast */
+				aa->argv = erealloc(aa->argv,
+				    (aa->size + 1) * sizeof(char *));
 			}
 
-			switch (ch = *++p) {
-			case '\0':
-			case '\n':
-				/* hmmm; fix it up as best we can */
-				ch = '\\';
-				--p;
-				break;
-			case 'b':
-				ch = '\b';
-				break;
-			case 'f':
-				ch = '\f';
-				break;
-			case 'n':
-				ch = '\n';
-				break;
-			case 'r':
-				ch = '\r';
-				break;
-			case 't':
-				ch = '\t';
-				break;
-			default:
+			*arg++ = '\0';
+			if (start == NULL) {
+				aa->argv[aa->argc] = start;
+				return;
+			}
+			if (str[0] == '\n' || str[0] == '\0') {
+				aa->argv[aa->argc++] = start;
+				aa->argv[aa->argc] = NULL;
+				return;
+			} else {
+				aa->argv[aa->argc++] = start;
+				start = NULL;
 				break;
 			}
+		case '\\':
+			if (start == NULL)
+				start = arg;
+			if (expand) {
+				switch (str[1]) {
+				case '\0':
+				case '\n':
+					/* hmmm; fix it up as best we can */
+					*arg++ = '\\';
+					break;
+				case 'b':
+					*arg++ = '\b';
+					++str;
+					break;
+				case 'f':
+					*arg++ = '\f';
+					++str;
+					break;
+				case 'n':
+					*arg++ = '\n';
+					++str;
+					break;
+				case 'r':
+					*arg++ = '\r';
+					++str;
+					break;
+				case 't':
+					*arg++ = '\t';
+					++str;
+					break;
+				default:
+					*arg++ = str[1];
+					++str;
+					break;
+				}
+			} else {
+				*arg++ = str[0];
+				++str;
+				*arg++ = str[0];
+			}
 			break;
 		default:
+			if (start == NULL)
+				start = arg;
+			*arg++ = str[0];
 			break;
 		}
-		if (!start)
-			start = t;
-		*t++ = (char)ch;
+		++str;
 	}
-done:	argv[argc] = NULL;
-	if (store_argc != NULL)
-		*store_argc = argc;
-	return (argv);
 }
 
 /*
@@ -271,23 +312,23 @@ MAKEFLAGS_quote(const char *str)
 	return (ret);
 }
 
-char **
-MAKEFLAGS_break(const char *str, int *pargc)
+void
+MAKEFLAGS_break(ArgArray *aa, const char str[])
 {
-	char *q, *start;
-	int len;
+	char	*arg;
+	char	*start;
+
+	ArgArray_Init(aa);
 
-	/* allocate room for a copy of the string */
-	if ((len = strlen(str) + 1) > curlen)
-		buffer = erealloc(buffer, curlen = len);
+	aa->buffer = strdup(str);
 
+	arg = aa->buffer;
 	start = NULL;
-	*pargc = 1;
 
-	for (q = buffer;;) {
-		switch (*str) {
-		  case ' ':
-		  case '\t':
+	for (;;) {
+		switch (str[0]) {
+		case ' ':
+		case '\t':
 			/* word separator */
 			if (start == NULL) {
 				/* not in a word */
@@ -295,41 +336,41 @@ MAKEFLAGS_break(const char *str, int *pargc)
 				continue;
 			}
 			/* FALLTHRU */
-		  case '\0':
-			if (start == NULL)
-				goto done;
-
-			/* finish word */
-			*q++ = '\0';
-			if (argmax == *pargc) {
-				argmax *= 2;
-				argv = erealloc(argv,
-				    sizeof(*argv) * (argmax + 1));
+		case '\0':
+			if (aa->argc == aa->size) {
+				aa->size *= 2;
+				aa->argv = erealloc(aa->argv,
+ 				    (aa->size + 1) * sizeof(char *));
 			}
-			argv[(*pargc)++] = start;
-			start = NULL;
 
-			if (*str++ == '\0')
-				goto done;
-			continue;
+			*arg++ = '\0';
+			if (start == NULL) {
+				aa->argv[aa->argc] = start;
+				return;
+			}
+			if (str[0] == '\0') {
+				aa->argv[aa->argc++] = start;
+				aa->argv[aa->argc] = NULL;
+				return;
+			} else {
+				aa->argv[aa->argc++] = start;
+				start = NULL;
+				str++;
+				continue;
+			}
 
-		  case '\\':
+		case '\\':
 			if (str[1] == ' ' || str[1] == '\t')
-				/* was a quote */
 				str++;
 			break;
 
-		  default:
+		default:
 			break;
 		}
 		if (start == NULL)
-			/* start of new word */
-			start = q;
-		*q++ = *str++;
+			start = arg;
+		*arg++ = *str++;
 	}
-  done:
-	argv[(*pargc)] = NULL;
-	return (argv);
 }
 
 /*
diff --git a/usr.bin/make/str.h b/usr.bin/make/str.h
index b50ed6c..8f74f76 100644
--- a/usr.bin/make/str.h
+++ b/usr.bin/make/str.h
@@ -45,6 +45,18 @@
 
 struct Buffer;
 
+/**
+ * An array of c-strings.  The pointers stored in argv, point to
+ * strings stored in buffer.
+ */
+typedef struct ArgArray {
+	int	size;		/* size of argv array */
+	int	argc;		/* strings referenced in argv */
+	char	**argv;		/* array of string pointers */
+	size_t	len;		/* size of buffer */
+	char	*buffer;	/* data buffer */
+} ArgArray;
+
 /*
  * These constants are all used by the Str_Concat function to decide how the
  * final string should look. If STR_ADDSPACE is given, a space will be
@@ -55,11 +67,12 @@ struct Buffer;
 #define	STR_ADDSPACE	0x01	/* add a space when Str_Concat'ing */
 #define	STR_ADDSLASH	0x04	/* add a slash when Str_Concat'ing */
 
-void str_init(void);
+void ArgArray_Done(ArgArray *);
+
 char *str_concat(const char *, const char *, int);
-char **brk_string(const char *, int *, Boolean);
+void brk_string(ArgArray *, const char [], Boolean);
 char *MAKEFLAGS_quote(const char *);
-char **MAKEFLAGS_break(const char *, int *);
+void MAKEFLAGS_break(ArgArray *, const char []);
 int Str_Match(const char *, const char *);
 const char *Str_SYSVMatch(const char *, const char *, int *);
 void Str_SYSVSubst(struct Buffer *, const char *, const char *, int);
diff --git a/usr.bin/make/var.c b/usr.bin/make/var.c
index 077ddbf..cb40a49 100644
--- a/usr.bin/make/var.c
+++ b/usr.bin/make/var.c
@@ -1178,21 +1178,23 @@ Var_Value(const char *name, GNode *ctxt, char **frp)
 static char *
 VarModify(const char *str, VarModifyProc *modProc, void *datum)
 {
-	char	**av;		/* word list [first word does not count] */
-	int	ac;
-	Buffer	*buf;		/* Buffer for the new string */
-	Boolean	addSpace;	/* TRUE if need to add a space to the buffer
-				 * before adding the trimmed word */
-	int	i;
-
-	av = brk_string(str, &ac, FALSE);
+	ArgArray	aa;
+	Buffer		*buf;		/* Buffer for the new string */
+	int		i;
+	Boolean		addSpace;	/*
+					 * TRUE if need to add a space to
+					 * the buffer before adding the
+					 * trimmed word
+					 */
 
-	buf = Buf_Init(0);
+	brk_string(&aa, str, FALSE);
 
 	addSpace = FALSE;
-	for (i = 1; i < ac; i++)
-		addSpace = (*modProc)(av[i], addSpace, buf, datum);
+	buf = Buf_Init(0);
+	for (i = 1; i < aa.argc; i++)
+		addSpace = (*modProc)(aa.argv[i], addSpace, buf, datum);
 
+	ArgArray_Done(&aa);
 	return (Buf_Peel(buf));
 }
 
@@ -1205,28 +1207,24 @@ VarModify(const char *str, VarModifyProc *modProc, void *datum)
  *
  * Results:
  *	A string containing the words sorted
- *
- * Side Effects:
- *	Uses brk_string() so it invalidates any previous call to
- *	brk_string().
  */
 static char *
 VarSortWords(const char *str, int (*cmp)(const void *, const void *))
 {
-	char	**av;
-	int	ac;
-	Buffer	*buf;
-	int	i;
+	ArgArray	aa;
+	Buffer		*buf;
+	int		i;
 
-	av = brk_string(str, &ac, FALSE);
-	qsort(av + 1, ac - 1, sizeof(char *), cmp);
+	brk_string(&aa, str, FALSE);
+	qsort(aa.argv + 1, aa.argc - 1, sizeof(char *), cmp);
 
 	buf = Buf_Init(0);
-	for (i = 1; i < ac; i++) {
-		Buf_Append(buf, av[i]);
-		Buf_AddByte(buf, (Byte)((i < ac - 1) ? ' ' : '\0'));
+	for (i = 1; i < aa.argc; i++) {
+		Buf_Append(buf, aa.argv[i]);
+		Buf_AddByte(buf, (Byte)((i < aa.argc - 1) ? ' ' : '\0'));
 	}
 
+	ArgArray_Done(&aa);
 	return (Buf_Peel(buf));
 }