Re: Patch to make for MAKEFLAGS quoting

[Max Okumoto <okumoto@xxxxxxxx>]

Joerg Sonnenberger <joerg@xxxxxxxxxxxxxxxxx> writes:
> On Mon, Jan 24, 2005 at 01:58:30PM +0100, Harti Brandt wrote:
>>
>> Hi,
>>
>> could one of you please test the attached patch for the quoting
>> problem in make(1)?
>>
>> harti
>
> The patch works fine. It even fixed another problem in the xorg build.
>
> Joerg

Thanks Harti!

I worked up a patch for the current dfly usr.bin/make code base.
Can more people test it?

				Max
diff -ru  fbsd-src/make/main.c dfly-src/make/main.c
--- fbsd-src/make/main.c	Tue Jan 25 06:18:15 2005
+++ dfly-src/make/main.c	Tue Jan 25 06:20:01 2005
@@ -141,14 +141,21 @@
 static void
 MFLAGS_append(const char *flag, char *arg)
 {
+	char *str;
 
 	Var_Append(MAKEFLAGS, flag, VAR_GLOBAL);
-	if (arg != NULL)
-		Var_Append(MAKEFLAGS, arg, VAR_GLOBAL);
+	if (arg != NULL) {
+		str = MAKEFLAGS_quote(arg);
+		Var_Append(MAKEFLAGS, str, VAR_GLOBAL);
+		free(str);
+	}
 
 	Var_Append("MFLAGS", flag, VAR_GLOBAL);
-	if (arg != NULL)
-		Var_Append("MFLAGS", arg, VAR_GLOBAL);
+	if (arg != NULL) {
+		str = MAKEFLAGS_quote(arg);
+		Var_Append("MFLAGS", str, VAR_GLOBAL);
+		free(str);
+	}
 }
 
 /*-
@@ -337,6 +344,11 @@
 	 */
 	for (argv += optind, argc -= optind; *argv; ++argv, --argc)
 		if (Parse_IsVar(*argv)) {
+			char *ptr = MAKEFLAGS_quote(*argv);
+
+			Var_Append(MAKEFLAGS, ptr, VAR_GLOBAL);
+			free(ptr);
+
 			Parse_DoVar(*argv, VAR_CMD);
 		} else {
 			if (!**argv)
@@ -368,7 +380,7 @@
  *	Only those that come from the various arguments.
  */
 void
-Main_ParseArgLine(char *line)
+Main_ParseArgLine(char *line, int mflags)
 {
 	char **argv;			/* Manufactured argument vector */
 	int argc;			/* Number of arguments in argv */
@@ -380,7 +392,11 @@
 	if (!*line)
 		return;
 
-	argv = brk_string(line, &argc, TRUE);
+	if (mflags)
+		argv = MAKEFLAGS_break(line, &argc);
+	else
+		argv = brk_string(line, &argc, TRUE);
+
 	MainParseArgs(argc, argv);
 }
 
@@ -615,7 +631,7 @@
 	 * (Note this is *not* MAKEFLAGS since /bin/make uses that and it's
 	 * in a different format).
 	 */
-	Main_ParseArgLine(getenv("MAKEFLAGS"));
+	Main_ParseArgLine(getenv("MAKEFLAGS"), 1);
 
 	MainParseArgs(argc, argv);
 
diff -ru  fbsd-src/make/nonints.h dfly-src/make/nonints.h
--- fbsd-src/make/nonints.h	Tue Jan 25 06:18:14 2005
+++ dfly-src/make/nonints.h	Tue Jan 25 06:20:01 2005
@@ -40,6 +40,8 @@
  */
 
 /* main.c */
-void Main_ParseArgLine(char *);
+void Main_ParseArgLine(char *, int);
+char *MAKEFLAGS_quote(const char *);
+char **MAKEFLAGS_break(const char *, int *);
 char *Cmd_Exec(const char *, const char **);
 #endif /* nonints_h_33c5dafb */
diff -ru  fbsd-src/make/parse.c dfly-src/make/parse.c
--- fbsd-src/make/parse.c	Tue Jan 25 06:18:15 2005
+++ dfly-src/make/parse.c	Tue Jan 25 06:20:01 2005
@@ -1043,7 +1043,7 @@
 	 * set the initial character to a null-character so the loop to
 	 * get sources won't get anything
 	 */
-	Main_ParseArgLine(line);
+	Main_ParseArgLine(line, 0);
 	*line = '\0';
     } else if (specType == ExShell) {
 	if (Job_ParseShell(line) != SUCCESS) {
diff -ru  fbsd-src/make/str.c dfly-src/make/str.c
--- fbsd-src/make/str.c	Tue Jan 25 06:18:15 2005
+++ dfly-src/make/str.c	Tue Jan 25 06:20:01 2005
@@ -227,6 +227,105 @@
 }
 
 /*
+ * Quote a string for appending it to MAKEFLAGS. According to Posix the
+ * kind of quoting here is implementation-defined. This quoting must ensure
+ * that the parsing of MAKEFLAGS's contents in a sub-shell yields the same
+ * options, option arguments and macro definitions as in the calling make.
+ * We simply quote all blanks, which according to Posix are space and tab
+ * in the POSIX locale. Don't use isblank because in that case makes with
+ * different locale settings could not communicate. We must also quote
+ * backslashes obviously.
+ */
+char *
+MAKEFLAGS_quote(const char *str)
+{
+	char *ret, *q;
+	const char *p;
+
+	/* assume worst case - everything has to be quoted */
+	ret = emalloc(strlen(str) * 2 + 1);
+
+	p = str;
+	q = ret;
+	while (*p != '\0') {
+		switch (*p) {
+
+		  case ' ':
+		  case '\t':
+			*q++ = '\\';
+			break;
+
+		  default:
+			break;
+		}
+		*q++ = *p++;
+	}
+	*q++ = '\0';
+	return (ret);
+}
+
+char **
+MAKEFLAGS_break(const char *str, int *pargc)
+{
+	char *q, *start;
+	int len;
+
+	/* allocate room for a copy of the string */
+	if ((len = strlen(str) + 1) > curlen)
+		buffer = erealloc(buffer, curlen = len);
+
+	start = NULL;
+	*pargc = 1;
+
+	for (q = buffer;;) {
+		switch (*str) {
+		  case ' ':
+		  case '\t':
+			/* word separator */
+			if (start == NULL) {
+				/* not in a word */
+				str++;
+				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));
+			}
+			argv[(*pargc)++] = start;
+			start = NULL;
+
+			if (*str++ == '\0')
+				goto done;
+			continue;
+
+		  case '\\':
+			if (str[1] == ' ' || str[1] == '\t')
+				/* was a quote */
+				str++;
+			break;
+
+		  default:
+			break;
+		}
+		if (start == NULL)
+			/* start of new word */
+			start = q;
+		*q++ = *str++;
+	}
+  done:
+	argv[(*pargc)] = NULL;
+	return (argv);
+}
+
+/*
  * Str_Match --
  *
  * See if a particular string matches a particular pattern.