On Sat, Apr 19, 2014 at 3:17 PM, Jeff King <peff@xxxxxxxx> wrote: > 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 s/just/just use/ > 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