Re: new module 'system-quote'

[Bruno Haible]

Hi Paul,

> > shell_quote_argv and system_quote_argv are essentially the same
> > function, differing only in that one invokes
> > shell_quote_length/shell_quote_copy and the other calls
> > system_quote_length/system_quote_copy.  Perhaps they should be
> > refactored into calling a single function that is parameterized by
> > quoting function.
> 
> I agree. The code has now reached a size where the code duplication
> makes future changes risky. I'll refactor as you say.

Refactoring done, as follows:


2012-05-10  Bruno Haible  <address@hidden>

        system-quote: Refactor.
        * lib/system-quote.h (system_quote_copy): Fix comment.
        * lib/system-quote.c (windows_createprocess_quote, windows_cmd_quote):
        New functions, extracted from system_quote_copy.
        (system_quote_length, system_quote_copy): Use these functions.
        Reported by Paul Eggert.

--- lib/system-quote.h.orig     Thu May 10 13:49:17 2012
+++ lib/system-quote.h  Wed May  9 23:41:23 2012
@@ -58,7 +58,7 @@
                             const char *string);
 
 /* Copies the quoted string to p and returns the incremented p.
-   There must be room for shell_quote_length (string) + 1 bytes at p.  */
+   There must be room for system_quote_length (string) + 1 bytes at p.  */
 extern char *
        system_quote_copy (char *p,
                           enum system_command_interpreter interpreter,
--- lib/system-quote.c.orig     Thu May 10 13:49:17 2012
+++ lib/system-quote.c  Thu May 10 00:05:33 2012
@@ -28,6 +28,7 @@
 #include "xalloc.h"
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+
 /* The native Windows CreateProcess() function interprets characters like
    ' ', '\t', '\\', '"' (but not '<' and '>') in a special way:
    - Space and tab are interpreted as delimiters. They are not treated as
@@ -46,6 +47,58 @@
  */
 # define SHELL_SPECIAL_CHARS "\"\\ 
\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037*"
 # define SHELL_SPACE_CHARS " 
\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037"
+
+/* Copies the quoted string to p and returns the number of bytes needed.
+   If p is non-NULL, there must be room for system_quote_length (string)
+   bytes at p.  */
+static size_t
+windows_createprocess_quote (char *p, const char *string)
+{
+  size_t len = strlen (string);
+  bool quote_around =
+    (len == 0 || strpbrk (string, SHELL_SPECIAL_CHARS) != NULL);
+  size_t backslashes = 0;
+  size_t i = 0;
+# define STORE(c) \
+  do                 \
+    {                \
+      if (p != NULL) \
+        p[i] = (c);  \
+      i++;           \
+    }                \
+  while (0)
+
+  if (quote_around)
+    STORE ('"');
+  for (; len > 0; string++, len--)
+    {
+      char c = *string;
+
+      if (c == '"')
+        {
+          size_t j;
+
+          for (j = backslashes + 1; j > 0; j--)
+            STORE ('\\');
+        }
+      STORE (c);
+      if (c == '\\')
+        backslashes++;
+      else
+        backslashes = 0;
+    }
+  if (quote_around)
+    {
+      size_t j;
+
+      for (j = backslashes; j > 0; j--)
+        STORE ('\\');
+      STORE ('"');
+    }
+# undef STORE
+  return i;
+}
+
 /* The native Windows cmd.exe command interpreter also interprets:
    - '\n', '\r' as a command terminator - no way to escape it,
    - '<', '>' as redirections,
@@ -61,6 +114,67 @@
  */
 # define CMD_SPECIAL_CHARS "\"\\ 
\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037!%&'*+,;<=>[]^`{|}~"
 # define CMD_FORBIDDEN_CHARS "\n\r"
+
+/* Copies the quoted string to p and returns the number of bytes needed.
+   If p is non-NULL, there must be room for system_quote_length (string)
+   bytes at p.  */
+static size_t
+windows_cmd_quote (char *p, const char *string)
+{
+  size_t len = strlen (string);
+  bool quote_around =
+    (len == 0 || strpbrk (string, CMD_SPECIAL_CHARS) != NULL);
+  size_t backslashes = 0;
+  size_t i = 0;
+# define STORE(c) \
+  do                 \
+    {                \
+      if (p != NULL) \
+        p[i] = (c);  \
+      i++;           \
+    }                \
+  while (0)
+
+  if (quote_around)
+    STORE ('"');
+  for (; len > 0; string++, len--)
+    {
+      char c = *string;
+
+      if (c == '"')
+        {
+          size_t j;
+
+          for (j = backslashes + 1; j > 0; j--)
+            STORE ('\\');
+        }
+      if (c == '%')
+        {
+          size_t j;
+
+          for (j = backslashes; j > 0; j--)
+            STORE ('\\');
+          STORE ('"');
+        }
+      STORE (c);
+      if (c == '%')
+        STORE ('"');
+      if (c == '\\')
+        backslashes++;
+      else
+        backslashes = 0;
+    }
+  if (quote_around)
+    {
+      size_t j;
+
+      for (j = backslashes; j > 0; j--)
+        STORE ('\\');
+      STORE ('"');
+    }
+  return i;
+}
+
 #endif
 
 size_t
@@ -77,59 +191,11 @@
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
     case SCI_WINDOWS_CREATEPROCESS:
-      {
-        size_t len = strlen (string);
-        bool quote_around =
-          (len == 0 || strpbrk (string, SHELL_SPECIAL_CHARS) != NULL);
-        size_t backslashes = 0;
-        size_t length = len;
-
-        if (quote_around)
-          length++;
-        for (; len > 0; string++, len--)
-          {
-            char c = *string;
-
-            if (c == '"')
-              length += backslashes + 1;
-            if (c == '\\')
-              backslashes++;
-            else
-              backslashes = 0;
-          }
-        if (quote_around)
-          length += backslashes + 1;
-        return length;
-      }
+      return windows_createprocess_quote (NULL, string);
 
     case SCI_SYSTEM:
     case SCI_WINDOWS_CMD:
-      {
-        size_t len = strlen (string);
-        bool quote_around =
-          (len == 0 || strpbrk (string, CMD_SPECIAL_CHARS) != NULL);
-        size_t backslashes = 0;
-        size_t length = len;
-
-        if (quote_around)
-          length++;
-        for (; len > 0; string++, len--)
-          {
-            char c = *string;
-
-            if (c == '"')
-              length += backslashes + 1;
-            if (c == '%')
-              length += backslashes + 2;
-            if (c == '\\')
-              backslashes++;
-            else
-              backslashes = 0;
-          }
-        if (quote_around)
-          length += backslashes + 1;
-        return length;
-      }
+      return windows_cmd_quote (NULL, string);
 #endif
 
     default:
@@ -153,91 +219,15 @@
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
     case SCI_WINDOWS_CREATEPROCESS:
-      {
-        size_t len = strlen (string);
-        bool quote_around =
-          (len == 0 || strpbrk (string, SHELL_SPECIAL_CHARS) != NULL);
-        size_t backslashes = 0;
-
-        if (quote_around)
-          *p++ = '"';
-        for (; len > 0; string++, len--)
-          {
-            char c = *string;
-
-            if (c == '"')
-              {
-                size_t j;
-
-                for (j = backslashes + 1; j > 0; j--)
-                  *p++ = '\\';
-              }
-            *p++ = c;
-            if (c == '\\')
-              backslashes++;
-            else
-              backslashes = 0;
-          }
-        if (quote_around)
-          {
-            size_t j;
-
-            for (j = backslashes; j > 0; j--)
-              *p++ = '\\';
-            *p++ = '"';
-          }
-        *p = '\0';
-        return p;
-      }
+      p += windows_createprocess_quote (p, string);
+      *p = '\0';
+      return p;
 
     case SCI_SYSTEM:
     case SCI_WINDOWS_CMD:
-      {
-        size_t len = strlen (string);
-        bool quote_around =
-          (len == 0 || strpbrk (string, CMD_SPECIAL_CHARS) != NULL);
-        size_t backslashes = 0;
-
-        if (quote_around)
-          *p++ = '"';
-        for (; len > 0; string++, len--)
-          {
-            char c = *string;
-
-            if (c == '"')
-              {
-                size_t j;
-
-                for (j = backslashes + 1; j > 0; j--)
-                  *p++ = '\\';
-              }
-            if (c == '%')
-              {
-                size_t j;
-
-                for (j = backslashes; j > 0; j--)
-                  *p++ = '\\';
-                *p++ = '"';
-              }
-            *p++ = c;
-            if (c == '%')
-              *p++ = '"';
-            if (c == '\\')
-              backslashes++;
-            else
-              backslashes = 0;
-          }
-        if (quote_around)
-          {
-            size_t j;
-
-            for (j = backslashes; j > 0; j--)
-              *p++ = '\\';
-            *p++ = '"';
-          }
-        *p = '\0';
-        return p;
-      }
+      p += windows_cmd_quote (p, string);
+      *p = '\0';
+      return p;
 #endif
 
     default: