One more note: at this moment the problem is slightly deeper. This array is next passed to the execvp function, which now falls with EFAULT on two my machines (both faced this problem after upgrading to ubuntu 14.04, everything 'worked' fine before, looks like now execvp checks input more strictly). This leads to non-working 'git difftool'. 2014-04-19 23:17 GMT+04:00, Jeff King <peff@xxxxxxxx>: > We currently generate the command-line for the external > command using a fixed-length array of size 10. But if there > is a rename, we actually need 11 elements (10 items, plus a > NULL), and end up writing a random NULL onto the stack. > > Rather than bump the limit, let's just an argv_array, which > makes this sort of error impossible. > > Noticed-by: Max L <infthi.inbox@xxxxxxxxx> > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > This was actually noticed by a GitHub user, who proposed bumping > the array size to 11: > > https://github.com/git/git/pull/92 > > Even though this fix is a bigger change, I'd rather do it this way, as > it is more obviously correct to a reader (and it solves the problem > forever). I pulled the name/email from that commit, but please let me > know if you'd prefer to be credited differently. > > diff.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/diff.c b/diff.c > index 539997f..b154284 100644 > --- a/diff.c > +++ b/diff.c > @@ -16,6 +16,7 @@ > #include "submodule.h" > #include "ll-merge.h" > #include "string-list.h" > +#include "argv-array.h" > > #ifdef NO_FAST_WORKING_DIRECTORY > #define FAST_WORKING_DIRECTORY 0 > @@ -2902,9 +2903,8 @@ static void run_external_diff(const char *pgm, > int complete_rewrite, > struct diff_options *o) > { > - const char *spawn_arg[10]; > + struct argv_array argv = ARGV_ARRAY_INIT; > int retval; > - const char **arg = &spawn_arg[0]; > struct diff_queue_struct *q = &diff_queued_diff; > const char *env[3] = { NULL }; > char env_counter[50]; > @@ -2915,23 +2915,22 @@ static void run_external_diff(const char *pgm, > const char *othername = (other ? other : name); > temp_one = prepare_temp_file(name, one); > temp_two = prepare_temp_file(othername, two); > - *arg++ = pgm; > - *arg++ = name; > - *arg++ = temp_one->name; > - *arg++ = temp_one->hex; > - *arg++ = temp_one->mode; > - *arg++ = temp_two->name; > - *arg++ = temp_two->hex; > - *arg++ = temp_two->mode; > + argv_array_push(&argv, pgm); > + argv_array_push(&argv, name); > + argv_array_push(&argv, temp_one->name); > + argv_array_push(&argv, temp_one->hex); > + argv_array_push(&argv, temp_one->mode); > + argv_array_push(&argv, temp_two->name); > + argv_array_push(&argv, temp_two->hex); > + argv_array_push(&argv, temp_two->mode); > if (other) { > - *arg++ = other; > - *arg++ = xfrm_msg; > + argv_array_push(&argv, other); > + argv_array_push(&argv, xfrm_msg); > } > } else { > - *arg++ = pgm; > - *arg++ = name; > + argv_array_push(&argv, pgm); > + argv_array_push(&argv, name); > } > - *arg = NULL; > fflush(NULL); > > env[0] = env_counter; > @@ -2940,8 +2939,9 @@ static void run_external_diff(const char *pgm, > env[1] = env_total; > snprintf(env_total, sizeof(env_total), "GIT_DIFF_PATH_TOTAL=%d", q->nr); > > - retval = run_command_v_opt_cd_env(spawn_arg, RUN_USING_SHELL, NULL, env); > + retval = run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env); > remove_tempfile(); > + argv_array_clear(&argv); > if (retval) { > fprintf(stderr, "external diff died, stopping at %s.\n", name); > exit(1); > -- > 1.9.1.656.ge8a0637 > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html