This is the mail archive of the libc-hacker@sourceware.cygnus.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

wordexp is broken


Hi again,

Field-splitting was broken for command substitution. Here's a patch that
fixes it, and also splits ':abc:' into three fields not two (which is what
happened before, owing to bugs).

Tim.
*/

Sun Sep 13 14:53:50 1998  Tim Waugh  <tim@cyberelk.demon.co.uk>

	* posix/wordexp-test.c: Field-splitting ':abc:' with IFS=: should
	yield three fields, not two.  Test both parameter expansion and
	command substitution for correct field-splitting behaviour.

	* posix/wordexp.c (w_emptyword): New function.
	(parse_param): Use it.
	(exec_comm): Likewise, for consistency with the way parse_param
	splits fields.
	(parse_param): Fix some memory leaks.

--- /home/tim/cvs/libc/posix/wordexp-test.c	Sun Sep 13 00:26:31 1998
+++ ../libc/posix/wordexp-test.c	Sun Sep 13 14:53:07 1998
@@ -51,8 +51,19 @@
     { 0, "two three", "one $var", 0, 3, { "one", "two", "three", }, IFS },
     { 0, "two three", "one \"$var\"", 0, 2, { "one", "two three", }, "" },
     { 0, "two three", "one $var", 0, 2, { "one", "two three", }, "" },
-    { 0, ":abc:", "$var", 0, 2, { "", "abc", }, ":" }, /* cf. bash */
+    { 0, ":abc:", "$var", 0, 3, { "", "abc", "", }, ":" },
+    { 0, NULL, "$(echo :abc:)", 0, 3, { "", "abc", "", }, ":" },
+    { 0, NULL, "$(echo :abc:\\ )", 0, 3, { "", "abc", "", }, ": " },
+    { 0, NULL, "$(echo :abc\\ )", 0, 2, { "", "abc", }, ": " },
+    { 0, ":abc:", "$(echo $var)", 0, 3, { "", "abc", "", }, ":" },
     { 0, NULL, ":abc:", 0, 1, { " abc ", }, ":" },
+    { 0, NULL, "$(echo :abc:)def", 0, 3, { "", "abc", "def", }, ":" },
+    { 0, NULL, "$(echo abc:de)f", 0, 2, { "abc", "def", }, ":" },
+    { 0, NULL, "$(echo abc:de)f:ghi", 0, 2, { "abc", "def ghi", }, ":" },
+    { 0, "abc:", "$var$(echo def:ghi)", 0, 3, { "abc", "def", "ghi", }, ":" },
+    { 0, "abc:d", "$var$(echo ef:ghi)", 0, 3, { "abc", "def", "ghi", }, ":" },
+    { 0, "def:ghi", "$(echo abc:)$var", 0, 3, { "abc", "def", "ghi", }, ":" },
+    { 0, "ef:ghi", "$(echo abc:d)$var", 0, 3, { "abc", "def", "ghi", }, ":" },
 
     /* Simple parameter expansion */
     { 0, "foo", "${var}", 0, 1, { "foo", }, IFS },
--- /home/tim/cvs/libc/posix/wordexp.c	Sun Sep 13 00:26:32 1998
+++ ../libc/posix/wordexp.c	Sun Sep 13 14:55:42 1998
@@ -74,6 +74,7 @@
 
 #define W_CHUNK	(100)
 
+/* Result of w_newword will be ignored if it the last word. */
 static inline char *
 w_newword (size_t *actlen, size_t *maxlen)
 {
@@ -136,6 +137,20 @@
 }
 
 
+/* Result of w_emptyword will not be ignored even if it is the last. */
+static inline char *
+w_emptyword (size_t *actlen, size_t *maxlen)
+{
+  char *word = malloc (1 + W_CHUNK);
+  *maxlen = W_CHUNK;
+  *actlen = 0;
+
+  if (word)
+    *word = '\0';
+
+  return word;
+}
+
 static char *
 internal_function
 w_addstr (char *buffer, size_t *actlen, size_t *maxlen, const char *str)
@@ -811,6 +826,7 @@
   int i;
   char *buffer;
   pid_t pid;
+  int keep_empty_word = 0;
 
   /* Don't fork() unless necessary */
   if (!comm || !*comm)
@@ -911,6 +927,11 @@
 		  if (strchr (ifs_white, buffer[i]) == NULL)
 		    {
 		      /* Current character is IFS but not whitespace */
+
+		      /* After this delimiter, another field must result.
+		       * Make a note. */
+		      keep_empty_word = 1;
+
 		      if (copying == 2)
 			{
 			  /*            current character
@@ -935,25 +956,12 @@
 		      if (copying != 1)
 			continue;
 
-		      /* End of field (search for non-IFS afterwards) */
+		      /* End of field (search for non-ws IFS afterwards) */
 		      copying = 2;
 		    }
 
 		  /* First IFS white space, or IFS non-whitespace.
-		   * Delimit the field. */
-		  if (!*word)
-		    {
-		      /* This field is null, so make it an empty string */
-		      *word = w_addchar (*word, word_length, max_length, 0);
-		      if (*word == NULL)
-			{
-			  __kill (pid, SIGKILL);
-			  __waitpid (pid, NULL, 0);
-			  __close (fildes[0]);
-			  return WRDE_NOSPACE;
-			}
-		    }
-
+		   * Delimit the field.  Nulls are converted by w_addword. */
 		  if (w_addword (pwordexp, *word) == WRDE_NOSPACE)
 		    {
 		      __kill (pid, SIGKILL);
@@ -962,13 +970,20 @@
 		      return WRDE_NOSPACE;
 		    }
 
-		  *word = w_newword (word_length, max_length);
+		  if (keep_empty_word)
+		    *word = w_emptyword (word_length, max_length);
+		  else
+		    *word = w_newword (word_length, max_length);
 		  /* fall back round the loop.. */
 		}
 	      else
 		{
 		  /* Not IFS character */
 		  copying = 1;
+
+		  if (buffer[i] != '\n')
+		    keep_empty_word = 0;
+
 		  *word = w_addchar (*word, word_length, max_length,
 				     buffer[i]);
 		  if (*word == NULL)
@@ -985,7 +1000,18 @@
 
   /* Bash chops off trailing newlines, which seems sensible.  */
   while (*word_length > 0 && (*word)[*word_length - 1] == '\n')
-    (*word)[--*word_length] = '\0';
+    {
+      (*word)[--*word_length] = '\0';
+
+      /* If the last word was entirely newlines, and the previous word
+       * wasn't delimited with IFS non-whitespace, turn it into a new word
+       * which can be ignored if there's nothing following it. */
+      if (!keep_empty_word && *word_length == 0)
+	{
+	  free (*word);
+	  *word = w_newword (word_length, max_length);
+	}
+    }
 
   __close (fildes[0]);
   return 0;
@@ -1723,7 +1749,7 @@
 	free (value);
 
       if (value_copy == NULL)
-	return WRDE_NOSPACE;
+	goto no_space;
 
       do
 	{
@@ -1736,10 +1762,10 @@
 	      if (w_addword (pwordexp, *word) == WRDE_NOSPACE)
 		{
 		  free (value_copy);
-		  return WRDE_NOSPACE;
+		  goto no_space;
 		}
 
-	      *word = w_newword (word_length, max_length);
+	      *word = w_emptyword (word_length, max_length);
 	    }
 
 	  /* Skip IFS whitespace before the field */
@@ -1773,7 +1799,7 @@
 	  if (*word == NULL && *field_begin != '\0')
 	    {
 	      free (value_copy);
-	      return WRDE_NOSPACE;
+	      goto no_space;
 	    }
 
 	  field_begin = next_field;
@@ -2217,7 +2243,7 @@
   /* End of string */
 
   /* There was a word separator at the end */
-  if (word == NULL)
+  if (word == NULL) /* i.e. w_newword */
     return 0;
 
   /* There was no field separator at the end */



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]