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");
