Re: [PATCH 1/6] run_external_diff: use an argv_array for the command line

> 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@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> 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