Merge branch 'jk/external-diff-use-argv-array'
authorJunio C Hamano <gitster@pobox.com>
Tue, 3 Jun 2014 19:06:42 +0000 (12:06 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 3 Jun 2014 19:06:43 +0000 (12:06 -0700)
Code clean-up (and a bugfix which has been merged for 2.0).

* jk/external-diff-use-argv-array:
run_external_diff: refactor cmdline setup logic
run_external_diff: hoist common bits out of conditional
run_external_diff: drop fflush(NULL)
run_external_diff: clean up error handling
run_external_diff: use an argv_array for the environment

1  2 
diff.c
diff --combined diff.c
index 36679aaf4eab872386a29ee5502fd7282dcbf0a7,680f52db4b867f4dab42b794e0af706e5545f581..f66716fab4b477c58434126b4aaa4b4372ce163a
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -1362,7 -1362,11 +1362,7 @@@ static struct diffstat_file *diffstat_a
  {
        struct diffstat_file *x;
        x = xcalloc(sizeof (*x), 1);
 -      if (diffstat->nr == diffstat->alloc) {
 -              diffstat->alloc = alloc_nr(diffstat->alloc);
 -              diffstat->files = xrealloc(diffstat->files,
 -                              diffstat->alloc * sizeof(x));
 -      }
 +      ALLOC_GROW(diffstat->files, diffstat->nr + 1, diffstat->alloc);
        diffstat->files[diffstat->nr++] = x;
        if (name_b) {
                x->from_name = xstrdup(name_a);
@@@ -1462,12 -1466,20 +1462,12 @@@ int print_stat_summary(FILE *fp, int fi
         * but nothing about added/removed lines? Is this a bug in Git?").
         */
        if (insertions || deletions == 0) {
 -              /*
 -               * TRANSLATORS: "+" in (+) is a line addition marker;
 -               * do not translate it.
 -               */
                strbuf_addf(&sb,
                            (insertions == 1) ? ", %d insertion(+)" : ", %d insertions(+)",
                            insertions);
        }
  
        if (deletions || insertions == 0) {
 -              /*
 -               * TRANSLATORS: "-" in (-) is a line removal marker;
 -               * do not translate it.
 -               */
                strbuf_addf(&sb,
                            (deletions == 1) ? ", %d deletion(-)" : ", %d deletions(-)",
                            deletions);
@@@ -2880,6 -2892,16 +2880,16 @@@ static struct diff_tempfile *prepare_te
        return temp;
  }
  
+ static void add_external_diff_name(struct argv_array *argv,
+                                  const char *name,
+                                  struct diff_filespec *df)
+ {
+       struct diff_tempfile *temp = prepare_temp_file(name, df);
+       argv_array_push(argv, temp->name);
+       argv_array_push(argv, temp->hex);
+       argv_array_push(argv, temp->mode);
+ }
  /* An external diff command takes:
   *
   * diff-cmd name infile1 infile1-sha1 infile1-mode \
@@@ -2896,48 -2918,32 +2906,32 @@@ static void run_external_diff(const cha
                              struct diff_options *o)
  {
        struct argv_array argv = ARGV_ARRAY_INIT;
-       int retval;
+       struct argv_array env = ARGV_ARRAY_INIT;
        struct diff_queue_struct *q = &diff_queued_diff;
-       const char *env[3] = { NULL };
-       char env_counter[50];
-       char env_total[50];
+       argv_array_push(&argv, pgm);
+       argv_array_push(&argv, name);
  
        if (one && two) {
-               struct diff_tempfile *temp_one, *temp_two;
-               const char *othername = (other ? other : name);
-               temp_one = prepare_temp_file(name, one);
-               temp_two = prepare_temp_file(othername, two);
-               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) {
+               add_external_diff_name(&argv, name, one);
+               if (!other)
+                       add_external_diff_name(&argv, name, two);
+               else {
+                       add_external_diff_name(&argv, other, two);
                        argv_array_push(&argv, other);
                        argv_array_push(&argv, xfrm_msg);
                }
-       } else {
-               argv_array_push(&argv, pgm);
-               argv_array_push(&argv, name);
        }
-       fflush(NULL);
  
-       env[0] = env_counter;
-       snprintf(env_counter, sizeof(env_counter), "GIT_DIFF_PATH_COUNTER=%d",
-                ++o->diff_path_counter);
-       env[1] = env_total;
-       snprintf(env_total, sizeof(env_total), "GIT_DIFF_PATH_TOTAL=%d", q->nr);
+       argv_array_pushf(&env, "GIT_DIFF_PATH_COUNTER=%d", ++o->diff_path_counter);
+       argv_array_pushf(&env, "GIT_DIFF_PATH_TOTAL=%d", q->nr);
+       if (run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env.argv))
+               die(_("external diff died, stopping at %s"), name);
  
-       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);
-       }
+       argv_array_clear(&env);
  }
  
  static int similarity_index(struct diff_filepair *p)
@@@ -3205,7 -3211,6 +3199,7 @@@ void diff_setup(struct diff_options *op
        options->context = diff_context_default;
        DIFF_OPT_SET(options, RENAME_EMPTY);
  
 +      /* pathchange left =NULL by default */
        options->change = diff_change;
        options->add_remove = diff_addremove;
        options->use_color = diff_use_color_default;
@@@ -3355,11 -3360,14 +3349,11 @@@ static int opt_arg(const char *arg, in
        if (c != '-')
                return 0;
        arg++;
 -      eq = strchr(arg, '=');
 -      if (eq)
 -              len = eq - arg;
 -      else
 -              len = strlen(arg);
 +      eq = strchrnul(arg, '=');
 +      len = eq - arg;
        if (!len || strncmp(arg, arg_long, len))
                return 0;
 -      if (eq) {
 +      if (*eq) {
                int n;
                char *end;
                if (!isdigit(*++eq))
@@@ -3586,6 -3594,14 +3580,6 @@@ static int parse_diff_filter_opt(const 
        return 0;
  }
  
 -/* Used only by "diff-files" and "diff --no-index" */
 -void handle_deprecated_show_diff_q(struct diff_options *opt)
 -{
 -      warning("'diff -q' and 'diff-files -q' are deprecated.");
 -      warning("Use 'diff --diff-filter=d' instead to ignore deleted filepairs.");
 -      parse_diff_filter_opt("d", opt);
 -}
 -
  static void enable_patch_output(int *fmt) {
        *fmt &= ~DIFF_FORMAT_NO_OUTPUT;
        *fmt |= DIFF_FORMAT_PATCH;
@@@ -3944,7 -3960,11 +3938,7 @@@ struct diff_queue_struct diff_queued_di
  
  void diff_q(struct diff_queue_struct *queue, struct diff_filepair *dp)
  {
 -      if (queue->alloc <= queue->nr) {
 -              queue->alloc = alloc_nr(queue->alloc);
 -              queue->queue = xrealloc(queue->queue,
 -                                      sizeof(dp) * queue->alloc);
 -      }
 +      ALLOC_GROW(queue->queue, queue->nr + 1, queue->alloc);
        queue->queue[queue->nr++] = dp;
  }
  
@@@ -4750,7 -4770,6 +4744,7 @@@ void diffcore_fix_diff_index(struct dif
  
  void diffcore_std(struct diff_options *options)
  {
 +      /* NOTE please keep the following in sync with diff_tree_combined() */
        if (options->skip_stat_unmatch)
                diffcore_skip_stat_unmatch(options);
        if (!options->found_follow) {