Re: new module 'system-quote'

[Paul Eggert]

Thanks for working on this.  Some comments:

> /* Copies the quoted string to p and returns the incremented p.
>    There must be room for shell_quote_length (string) + 1 bytes at p.  */

Shouldn't that be system_quote_length (interpreter, string) + 1?

> # define CMD_FORBIDDEN_CHARS "\n\r"

That macro is never used -- is that intended?  Some compilers warn
about that sort of thing.  Similarly, SHELL_SPACE_CHARS is defined but
never used.

Are there restrictions about what can go into the strings?
For example, can they contain UTF-8 encoding errors?
This shouldn't be an issue with diffutils, but might be in other apps.

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.  This refactoring would be easier if we changed the
signature of system_quote_copy to be this:

  size_t system_quote_copy (char *p, const char *string);

so that it returns the length of the string that it creates, and it
doesn't store anything if !P.  That way, we need just one quoting
function, which we can pass as an argument to the refactored function
mentioned above.  E.g., instead of this:

  size_t length = system_quote_length (interpreter, str) + 2;
  char *buf = xmalloc (length);
  char *p = system_quote_copy (buf, interpreter, str);
  strcpy (p, "\n");

one would write this:

  size_t length = system_quote_copy (NULL, interpreter, str) + 2;
  char *buf = xmalloc (length);
  char *p = buf + system_quote_copy (buf, interpreter, str);
  strcpy (p, "\n");