A useful shell safety helper sq_expand() was hidden as a static function in diff.c. Extract it out and make it available as sq_quote().
Signed-off-by: Junio C Hamano <[EMAIL PROTECTED]> --- >>>>> "JCH" == Junio C Hamano <[EMAIL PROTECTED]> writes: JCH> I am not going to do it myself immediately so there is no worry JCH> to race against me ;-). Well, I ended up doing it myself just as a diversion from thinking about pulls ;-). As you said, one _must_ be very careful when feeding a shell, but being careful does not necessarily hard. This is the first part; the next one will depend on it. Makefile | 3 +++ diff.c | 47 ++++++----------------------------------------- 2 files changed, 9 insertions(+), 41 deletions(-) 0ab0d4c45dc7c531dd54f2d29a2199d4f4192018 diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -65,6 +65,9 @@ LIB_H=cache.h object.h blob.h tree.h com LIB_H += strbuf.h LIB_OBJS += strbuf.o +LIB_H += quote.h +LIB_OBJS += quote.o + LIB_H += diff.h count-delta.h LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \ count-delta.o diffcore-break.o diffcore-order.o diff --git a/diff.c b/diff.c --- a/diff.c +++ b/diff.c @@ -5,6 +5,7 @@ #include <sys/wait.h> #include <signal.h> #include "cache.h" +#include "quote.h" #include "diff.h" #include "diffcore.h" @@ -40,42 +41,6 @@ static const char *external_diff(void) return external_diff_cmd; } -/* Help to copy the thing properly quoted for the shell safety. - * any single quote is replaced with '\'', and the caller is - * expected to enclose the result within a single quote pair. - * - * E.g. - * original sq_expand result - * name ==> name ==> 'name' - * a b ==> a b ==> 'a b' - * a'b ==> a'\''b ==> 'a'\''b' - */ -static char *sq_expand(const char *src) -{ - static char *buf = NULL; - int cnt, c; - const char *cp; - char *bp; - - /* count bytes needed to store the quoted string. */ - for (cnt = 1, cp = src; *cp; cnt++, cp++) - if (*cp == '\'') - cnt += 3; - - buf = xmalloc(cnt); - bp = buf; - while ((c = *src++)) { - if (c != '\'') - *bp++ = c; - else { - bp = strcpy(bp, "'\\''"); - bp += 4; - } - } - *bp = 0; - return buf; -} - static struct diff_tempfile { const char *name; /* filename external diff should read from */ char hex[41]; @@ -167,16 +132,16 @@ static void builtin_diff(const char *nam int complete_rewrite) { int i, next_at, cmd_size; - const char *diff_cmd = "diff -L'%s%s' -L'%s%s'"; - const char *diff_arg = "'%s' '%s'||:"; /* "||:" is to return 0 */ + const char *diff_cmd = "diff -L%s%s -L%s%s"; + const char *diff_arg = "%s %s||:"; /* "||:" is to return 0 */ const char *input_name_sq[2]; const char *path0[2]; const char *path1[2]; const char *name_sq[2]; char *cmd; - name_sq[0] = sq_expand(name_a); - name_sq[1] = sq_expand(name_b); + name_sq[0] = sq_quote(name_a); + name_sq[1] = sq_quote(name_b); /* diff_cmd and diff_arg have 6 %s in total which makes * the sum of these strings 12 bytes larger than required. @@ -186,7 +151,7 @@ static void builtin_diff(const char *nam cmd_size = (strlen(diff_cmd) + strlen(diff_opts) + strlen(diff_arg) - 9); for (i = 0; i < 2; i++) { - input_name_sq[i] = sq_expand(temp[i].name); + input_name_sq[i] = sq_quote(temp[i].name); if (!strcmp(temp[i].name, "/dev/null")) { path0[i] = "/dev/null"; path1[i] = ""; - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html