Re: [patch] fix g_shell_quote

[Darin Adler <darin bentspoon com>]

On Thursday, August 16, 2001, at 06:57  AM, Havoc Pennington wrote:

> Looks good. Maybe also fix shell-test.c to trigger this?

Here's the fix along with some tests in shell-test.c. The rule about 
escape-sequence handling inside " marks in g_shell_unquote seems quite 
strange, by the way.

Index: glib/gshell.c
===================================================================
RCS file: /cvs/gnome/glib/glib/gshell.c,v
retrieving revision 1.3
diff -p -u -r1.3 gshell.c
--- glib/gshell.c	2001/03/09 21:23:33	1.3
+++ glib/gshell.c	2001/08/18 21:59:50
@@ -198,7 +198,7 @@ g_shell_quote (const gchar *unquoted_str
     {
       /* Replace literal ' with a close ', a \', and a open ' */
       if (*p == '\'')
-        g_string_append (dest, "'\''");
+        g_string_append (dest, "'\\''");
       else
         g_string_append_c (dest, *p);

Index: tests/shell-test.c
===================================================================
RCS file: /cvs/gnome/glib/tests/shell-test.c,v
retrieving revision 1.3
diff -p -u -r1.3 shell-test.c
--- tests/shell-test.c	2001/07/19 20:07:41	1.3
+++ tests/shell-test.c	2001/08/18 21:59:50
@@ -178,13 +178,113 @@ run_tests (void)
     }
 }

+static gboolean any_test_failed = FALSE;
+
+#define CHECK_STRING_RESULT(expression, expected_value) \
+ check_string_result (#expression, __FILE__, __LINE__, expression, 
expected_value)
+
+static void
+check_string_result (const char *expression,
+		     const char *file_name,
+		     int line_number,
+		     char *result,
+		     const char *expected)
+{
+	gboolean match;
+	
+	if (expected == NULL)
+	  match = result == NULL;
+	else
+	  match = result != NULL && strcmp (result, expected) == 0;
+
+	if (!match)
+	  {
+	    if (!any_test_failed)
+	      fprintf (stderr, "\n");
+	
+	    fprintf (stderr, "FAIL: check failed in %s, line %d\n", file_name, 
line_number);
+	    fprintf (stderr, "      evaluated: %s\n", expression);
+	    fprintf (stderr, "       expected: %s\n", expected == NULL ? 
"NULL" : expected);
+	    fprintf (stderr, "            got: %s\n", result == NULL ? 
"NULL" : result);
+	
+	    any_test_failed = TRUE;
+	  }
+
+	g_free (result);
+}
+
+static char *
+test_shell_unquote (const char *str)
+{
+  char *result;
+  GError *error;
+
+  error = NULL;
+  result = g_shell_unquote (str, &error);
+  if (error == NULL)
+    return result;
+
+  /* Leaks the error, which is no big deal. */
+
+  if (error->domain != G_SHELL_ERROR)
+    return g_strdup ("error in domain other than G_SHELL_ERROR");
+
+  /* It would be nice to check the error message too, but that's
+   * localized, so it's too much of a pain.
+   */
+  switch (error->code)
+    {
+    case G_SHELL_ERROR_BAD_QUOTING:
+      return g_strdup ("G_SHELL_ERROR_BAD_QUOTING");
+    case G_SHELL_ERROR_EMPTY_STRING:
+      return g_strdup ("G_SHELL_ERROR_EMPTY_STRING");
+    case G_SHELL_ERROR_FAILED:
+      return g_strdup ("G_SHELL_ERROR_FAILED");
+    default:
+      return g_strdup ("bad error code in G_SHELL_ERROR domain");
+    };
+}
+
 int
 main (int   argc,
       char *argv[])
 {
   run_tests ();

-  return 0;
-}
+  CHECK_STRING_RESULT (g_shell_quote (""), "''");
+  CHECK_STRING_RESULT (g_shell_quote ("a"), "'a'");
+  CHECK_STRING_RESULT (g_shell_quote ("("), "'('");
+  CHECK_STRING_RESULT (g_shell_quote ("'"), "''\\'''");
+  CHECK_STRING_RESULT (g_shell_quote ("'a"), "''\\''a'");
+  CHECK_STRING_RESULT (g_shell_quote ("a'"), "'a'\\'''");
+  CHECK_STRING_RESULT (g_shell_quote ("a'a"), "'a'\\''a'");
+
+  CHECK_STRING_RESULT (test_shell_unquote (""), "");
+  CHECK_STRING_RESULT (test_shell_unquote ("a"), "a");
+  CHECK_STRING_RESULT (test_shell_unquote ("'a'"), "a");
+  CHECK_STRING_RESULT (test_shell_unquote ("'('"), "(");
+  CHECK_STRING_RESULT (test_shell_unquote ("''\\'''"), "'");
+  CHECK_STRING_RESULT (test_shell_unquote ("''\\''a'"), "'a");
+  CHECK_STRING_RESULT (test_shell_unquote ("'a'\\'''"), "a'");
+  CHECK_STRING_RESULT (test_shell_unquote ("'a'\\''a'"), "a'a");

+  CHECK_STRING_RESULT (test_shell_unquote ("\\\\"), "\\");
+  CHECK_STRING_RESULT (test_shell_unquote ("\\\n"), "");

+  CHECK_STRING_RESULT (test_shell_unquote ("'\\''"), 
"G_SHELL_ERROR_BAD_QUOTING");
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\\"\""), "\"");
+
+  CHECK_STRING_RESULT (test_shell_unquote ("\""), 
"G_SHELL_ERROR_BAD_QUOTING");
+  CHECK_STRING_RESULT (test_shell_unquote ("'"), 
"G_SHELL_ERROR_BAD_QUOTING");
+
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\\\\""), "\\");
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\`\""), "`");
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\$\""), "$");
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\\n\""), "\n");
+
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\'\""), "\\'");
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\\r\""), "\\\r");
+  CHECK_STRING_RESULT (test_shell_unquote ("\"\\n\""), "\\n");
+
+  return any_test_failed ? 1 : 0;
+}
===================================================================

May I commit?

    -- Darin