Code

Merge branch 'jm/maint-diff-words-with-sbe'
authorJunio C Hamano <gitster@pobox.com>
Mon, 23 May 2011 17:27:42 +0000 (10:27 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 23 May 2011 17:27:42 +0000 (10:27 -0700)
* jm/maint-diff-words-with-sbe:
  do not read beyond end of malloc'd buffer

1  2 
diff.c

diff --combined diff.c
index ba5f7aa2173a7494b56395bfd88ab6228b5ed1fe,f90c7a896b814930952c42a7acde55fa8cf20e2b..8f4815bfd7d798f2a99131cd2e79155e933c7876
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -23,7 -23,7 +23,7 @@@
  #endif
  
  static int diff_detect_rename_default;
 -static int diff_rename_limit_default = 200;
 +static int diff_rename_limit_default = 400;
  static int diff_suppress_blank_empty;
  int diff_use_color_default = -1;
  static const char *diff_word_regex_cfg;
@@@ -31,7 -31,6 +31,7 @@@ static const char *external_diff_cmd_cf
  int diff_auto_refresh_index = 1;
  static int diff_mnemonic_prefix;
  static int diff_no_prefix;
 +static int diff_dirstat_permille_default = 30;
  static struct diff_options default_diff_options;
  
  static char diff_colors[][COLOR_MAXLEN] = {
@@@ -67,58 -66,6 +67,58 @@@ static int parse_diff_color_slot(const 
        return -1;
  }
  
 +static int parse_dirstat_params(struct diff_options *options, const char *params,
 +                              struct strbuf *errmsg)
 +{
 +      const char *p = params;
 +      int p_len, ret = 0;
 +
 +      while (*p) {
 +              p_len = strchrnul(p, ',') - p;
 +              if (!memcmp(p, "changes", p_len)) {
 +                      DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
 +                      DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
 +              } else if (!memcmp(p, "lines", p_len)) {
 +                      DIFF_OPT_SET(options, DIRSTAT_BY_LINE);
 +                      DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
 +              } else if (!memcmp(p, "files", p_len)) {
 +                      DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
 +                      DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
 +              } else if (!memcmp(p, "noncumulative", p_len)) {
 +                      DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
 +              } else if (!memcmp(p, "cumulative", p_len)) {
 +                      DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
 +              } else if (isdigit(*p)) {
 +                      char *end;
 +                      int permille = strtoul(p, &end, 10) * 10;
 +                      if (*end == '.' && isdigit(*++end)) {
 +                              /* only use first digit */
 +                              permille += *end - '0';
 +                              /* .. and ignore any further digits */
 +                              while (isdigit(*++end))
 +                                      ; /* nothing */
 +                      }
 +                      if (end - p == p_len)
 +                              options->dirstat_permille = permille;
 +                      else {
 +                              strbuf_addf(errmsg, _("  Failed to parse dirstat cut-off percentage '%.*s'\n"),
 +                                          p_len, p);
 +                              ret++;
 +                      }
 +              } else {
 +                      strbuf_addf(errmsg, _("  Unknown dirstat parameter '%.*s'\n"),
 +                                  p_len, p);
 +                      ret++;
 +              }
 +
 +              p += p_len;
 +
 +              if (*p)
 +                      p++; /* more parameters, swallow separator */
 +      }
 +      return ret;
 +}
 +
  static int git_config_rename(const char *var, const char *value)
  {
        if (!value)
@@@ -198,17 -145,6 +198,17 @@@ int git_diff_basic_config(const char *v
                return 0;
        }
  
 +      if (!strcmp(var, "diff.dirstat")) {
 +              struct strbuf errmsg = STRBUF_INIT;
 +              default_diff_options.dirstat_permille = diff_dirstat_permille_default;
 +              if (parse_dirstat_params(&default_diff_options, value, &errmsg))
 +                      warning(_("Found errors in 'diff.dirstat' config variable:\n%s"),
 +                              errmsg.buf);
 +              strbuf_release(&errmsg);
 +              diff_dirstat_permille_default = default_diff_options.dirstat_permille;
 +              return 0;
 +      }
 +
        if (!prefixcmp(var, "submodule."))
                return parse_submodule_config_option(var, value);
  
@@@ -309,15 -245,6 +309,15 @@@ static int fill_mmfile(mmfile_t *mf, st
        return 0;
  }
  
 +/* like fill_mmfile, but only for size, so we can avoid retrieving blob */
 +static unsigned long diff_filespec_size(struct diff_filespec *one)
 +{
 +      if (!DIFF_FILE_VALID(one))
 +              return 0;
 +      diff_populate_filespec(one, 1);
 +      return one->size;
 +}
 +
  static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
  {
        char *ptr = mf->ptr;
@@@ -645,14 -572,11 +645,14 @@@ static void emit_rewrite_diff(const cha
                line_prefix, metainfo, a_name.buf, name_a_tab, reset,
                line_prefix, metainfo, b_name.buf, name_b_tab, reset,
                line_prefix, fraginfo);
 -      print_line_count(o->file, lc_a);
 +      if (!o->irreversible_delete)
 +              print_line_count(o->file, lc_a);
 +      else
 +              fprintf(o->file, "?,?");
        fprintf(o->file, " +");
        print_line_count(o->file, lc_b);
        fprintf(o->file, " @@%s\n", reset);
 -      if (lc_a)
 +      if (lc_a && !o->irreversible_delete)
                emit_rewrite_lines(&ecbdata, '-', data_one, size_one);
        if (lc_b)
                emit_rewrite_lines(&ecbdata, '+', data_two, size_two);
@@@ -682,20 -606,22 +682,20 @@@ static void diff_words_append(char *lin
        buffer->text.ptr[buffer->text.size] = '\0';
  }
  
 -struct diff_words_style_elem
 -{
 +struct diff_words_style_elem {
        const char *prefix;
        const char *suffix;
        const char *color; /* NULL; filled in by the setup code if
                            * color is enabled */
  };
  
 -struct diff_words_style
 -{
 +struct diff_words_style {
        enum diff_words_type type;
        struct diff_words_style_elem new, old, ctx;
        const char *newline;
  };
  
 -struct diff_words_style diff_words_styles[] = {
 +static struct diff_words_style diff_words_styles[] = {
        { DIFF_WORDS_PORCELAIN, {"+", "\n"}, {"-", "\n"}, {" ", "\n"}, "~\n" },
        { DIFF_WORDS_PLAIN, {"{+", "+}"}, {"[-", "-]"}, {"", ""}, "\n" },
        { DIFF_WORDS_COLOR, {"", ""}, {"", ""}, {"", ""}, "\n" }
@@@ -1117,8 -1043,16 +1117,16 @@@ static void fn_out_consume(void *priv, 
                        emit_line(ecbdata->opt, plain, reset, line, len);
                        fputs("~\n", ecbdata->opt->file);
                } else {
-                       /* don't print the prefix character */
-                       emit_line(ecbdata->opt, plain, reset, line+1, len-1);
+                       /*
+                        * Skip the prefix character, if any.  With
+                        * diff_suppress_blank_empty, there may be
+                        * none.
+                        */
+                       if (line[0] != '\n') {
+                             line++;
+                             len--;
+                       }
+                       emit_line(ecbdata->opt, plain, reset, line, len);
                }
                return;
        }
@@@ -1309,7 -1243,7 +1317,7 @@@ static void show_stats(struct diffstat_
        uintmax_t max_change = 0, max_len = 0;
        int total_files = data->nr;
        int width, name_width;
 -      const char *reset, *set, *add_c, *del_c;
 +      const char *reset, *add_c, *del_c;
        const char *line_prefix = "";
        struct strbuf *msg = NULL;
  
  
        /* Find the longest filename and max number of changes */
        reset = diff_get_color_opt(options, DIFF_RESET);
 -      set   = diff_get_color_opt(options, DIFF_PLAIN);
        add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
        del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
  
@@@ -1519,7 -1454,7 +1527,7 @@@ struct dirstat_file 
  
  struct dirstat_dir {
        struct dirstat_file *files;
 -      int alloc, nr, percent, cumulative;
 +      int alloc, nr, permille, cumulative;
  };
  
  static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
         *    under this directory (sources == 1).
         */
        if (baselen && sources != 1) {
 -              int permille = this_dir * 1000 / changed;
 -              if (permille) {
 -                      int percent = permille / 10;
 -                      if (percent >= dir->percent) {
 +              if (this_dir) {
 +                      int permille = this_dir * 1000 / changed;
 +                      if (permille >= dir->permille) {
                                fprintf(opt->file, "%s%4d.%01d%% %.*s\n", line_prefix,
 -                                      percent, permille % 10, baselen, base);
 +                                      permille / 10, permille % 10, baselen, base);
                                if (!dir->cumulative)
                                        return 0;
                        }
@@@ -1596,7 -1532,7 +1604,7 @@@ static void show_dirstat(struct diff_op
        dir.files = NULL;
        dir.alloc = 0;
        dir.nr = 0;
 -      dir.percent = options->dirstat_percent;
 +      dir.permille = options->dirstat_permille;
        dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE);
  
        changed = 0;
                struct diff_filepair *p = q->queue[i];
                const char *name;
                unsigned long copied, added, damage;
 +              int content_changed;
 +
 +              name = p->two->path ? p->two->path : p->one->path;
 +
 +              if (p->one->sha1_valid && p->two->sha1_valid)
 +                      content_changed = hashcmp(p->one->sha1, p->two->sha1);
 +              else
 +                      content_changed = 1;
 +
 +              if (!content_changed) {
 +                      /*
 +                       * The SHA1 has not changed, so pre-/post-content is
 +                       * identical. We can therefore skip looking at the
 +                       * file contents altogether.
 +                       */
 +                      damage = 0;
 +                      goto found_damage;
 +              }
  
 -              name = p->one->path ? p->one->path : p->two->path;
 +              if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE)) {
 +                      /*
 +                       * In --dirstat-by-file mode, we don't really need to
 +                       * look at the actual file contents at all.
 +                       * The fact that the SHA1 changed is enough for us to
 +                       * add this file to the list of results
 +                       * (with each file contributing equal damage).
 +                       */
 +                      damage = 1;
 +                      goto found_damage;
 +              }
  
                if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
                        diff_populate_filespec(p->one, 0);
                /*
                 * Original minus copied is the removed material,
                 * added is the new material.  They are both damages
 -               * made to the preimage. In --dirstat-by-file mode, count
 -               * damaged files, not damaged lines. This is done by
 -               * counting only a single damaged line per file.
 +               * made to the preimage.
 +               * If the resulting damage is zero, we know that
 +               * diffcore_count_changes() considers the two entries to
 +               * be identical, but since content_changed is true, we
 +               * know that there must have been _some_ kind of change,
 +               * so we force all entries to have damage > 0.
                 */
                damage = (p->one->size - copied) + added;
 -              if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE) && damage > 0)
 +              if (!damage)
                        damage = 1;
  
 +found_damage:
                ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);
                dir.files[dir.nr].name = name;
                dir.files[dir.nr].changed = damage;
        gather_dirstat(options, &dir, changed, "", 0);
  }
  
 +static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *options)
 +{
 +      int i;
 +      unsigned long changed;
 +      struct dirstat_dir dir;
 +
 +      if (data->nr == 0)
 +              return;
 +
 +      dir.files = NULL;
 +      dir.alloc = 0;
 +      dir.nr = 0;
 +      dir.permille = options->dirstat_permille;
 +      dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE);
 +
 +      changed = 0;
 +      for (i = 0; i < data->nr; i++) {
 +              struct diffstat_file *file = data->files[i];
 +              unsigned long damage = file->added + file->deleted;
 +              if (file->is_binary)
 +                      /*
 +                       * binary files counts bytes, not lines. Must find some
 +                       * way to normalize binary bytes vs. textual lines.
 +                       * The following heuristic assumes that there are 64
 +                       * bytes per "line".
 +                       * This is stupid and ugly, but very cheap...
 +                       */
 +                      damage = (damage + 63) / 64;
 +              ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);
 +              dir.files[dir.nr].name = file->name;
 +              dir.files[dir.nr].changed = damage;
 +              changed += damage;
 +              dir.nr++;
 +      }
 +
 +      /* This can happen even with many files, if everything was renames */
 +      if (!changed)
 +              return;
 +
 +      /* Show all directories with more than x% of the changes */
 +      qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare);
 +      gather_dirstat(options, &dir, changed, "", 0);
 +}
 +
  static void free_diffstat_info(struct diffstat_t *diffstat)
  {
        int i;
@@@ -2091,11 -1951,7 +2099,11 @@@ static void builtin_diff(const char *na
                }
        }
  
 -      if (!DIFF_OPT_TST(o, TEXT) &&
 +      if (o->irreversible_delete && lbl[1][0] == '/') {
 +              fprintf(o->file, "%s", header.buf);
 +              strbuf_reset(&header);
 +              goto free_ab_and_return;
 +      } else if (!DIFF_OPT_TST(o, TEXT) &&
            ( (!textconv_one && diff_filespec_is_binary(one)) ||
              (!textconv_two && diff_filespec_is_binary(two)) )) {
                if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                        fprintf(o->file, "%sBinary files %s and %s differ\n",
                                line_prefix, lbl[0], lbl[1]);
                o->found_changes = 1;
 -      }
 -      else {
 +      } else {
                /* Crazy xdl interfaces.. */
                const char *diffopts = getenv("GIT_DIFF_OPTS");
                xpparam_t xpp;
@@@ -2230,28 -2087,25 +2238,28 @@@ static void builtin_diffstat(const cha
                data->is_unmerged = 1;
                return;
        }
 -      if (complete_rewrite) {
 +
 +      if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
 +              data->is_binary = 1;
 +              data->added = diff_filespec_size(two);
 +              data->deleted = diff_filespec_size(one);
 +      }
 +
 +      else if (complete_rewrite) {
                diff_populate_filespec(one, 0);
                diff_populate_filespec(two, 0);
                data->deleted = count_lines(one->data, one->size);
                data->added = count_lines(two->data, two->size);
 -              goto free_and_return;
        }
 -      if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 -              die("unable to read files to diff");
  
 -      if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
 -              data->is_binary = 1;
 -              data->added = mf2.size;
 -              data->deleted = mf1.size;
 -      } else {
 +      else {
                /* Crazy xdl interfaces.. */
                xpparam_t xpp;
                xdemitconf_t xecfg;
  
 +              if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 +                      die("unable to read files to diff");
 +
                memset(&xpp, 0, sizeof(xpp));
                memset(&xecfg, 0, sizeof(xecfg));
                xpp.flags = o->xdl_opts;
                              &xpp, &xecfg);
        }
  
 - free_and_return:
        diff_free_filespec_data(one);
        diff_free_filespec_data(two);
  }
@@@ -2998,7 -2853,7 +3006,7 @@@ void diff_setup(struct diff_options *op
        options->line_termination = '\n';
        options->break_opt = -1;
        options->rename_limit = -1;
 -      options->dirstat_percent = 3;
 +      options->dirstat_permille = diff_dirstat_permille_default;
        options->context = 3;
  
        options->change = diff_change;
@@@ -3256,21 -3111,6 +3264,21 @@@ static int stat_opt(struct diff_option
        return argcount;
  }
  
 +static int parse_dirstat_opt(struct diff_options *options, const char *params)
 +{
 +      struct strbuf errmsg = STRBUF_INIT;
 +      if (parse_dirstat_params(options, params, &errmsg))
 +              die(_("Failed to parse --dirstat/-X option parameter:\n%s"),
 +                  errmsg.buf);
 +      strbuf_release(&errmsg);
 +      /*
 +       * The caller knows a dirstat-related option is given from the command
 +       * line; allow it to say "return this_function();"
 +       */
 +      options->output_format |= DIFF_FORMAT_DIRSTAT;
 +      return 1;
 +}
 +
  int diff_opt_parse(struct diff_options *options, const char **av, int ac)
  {
        const char *arg = av[0];
                options->output_format |= DIFF_FORMAT_NUMSTAT;
        else if (!strcmp(arg, "--shortstat"))
                options->output_format |= DIFF_FORMAT_SHORTSTAT;
 -      else if (opt_arg(arg, 'X', "dirstat", &options->dirstat_percent))
 -              options->output_format |= DIFF_FORMAT_DIRSTAT;
 -      else if (!strcmp(arg, "--cumulative")) {
 -              options->output_format |= DIFF_FORMAT_DIRSTAT;
 -              DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
 -      } else if (opt_arg(arg, 0, "dirstat-by-file",
 -                         &options->dirstat_percent)) {
 -              options->output_format |= DIFF_FORMAT_DIRSTAT;
 -              DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
 +      else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
 +              return parse_dirstat_opt(options, "");
 +      else if (!prefixcmp(arg, "-X"))
 +              return parse_dirstat_opt(options, arg + 2);
 +      else if (!prefixcmp(arg, "--dirstat="))
 +              return parse_dirstat_opt(options, arg + 10);
 +      else if (!strcmp(arg, "--cumulative"))
 +              return parse_dirstat_opt(options, "cumulative");
 +      else if (!strcmp(arg, "--dirstat-by-file"))
 +              return parse_dirstat_opt(options, "files");
 +      else if (!prefixcmp(arg, "--dirstat-by-file=")) {
 +              parse_dirstat_opt(options, "files");
 +              return parse_dirstat_opt(options, arg + 18);
        }
        else if (!strcmp(arg, "--check"))
                options->output_format |= DIFF_FORMAT_CHECKDIFF;
                        return error("invalid argument to -M: %s", arg+2);
                options->detect_rename = DIFF_DETECT_RENAME;
        }
 +      else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
 +              options->irreversible_delete = 1;
 +      }
        else if (!prefixcmp(arg, "-C") || !prefixcmp(arg, "--find-copies=") ||
                 !strcmp(arg, "--find-copies")) {
                if (options->detect_rename == DIFF_DETECT_COPY)
@@@ -4122,34 -3955,11 +4130,34 @@@ static int is_summary_empty(const struc
        return 1;
  }
  
 +static const char rename_limit_warning[] =
 +"inexact rename detection was skipped due to too many files.";
 +
 +static const char degrade_cc_to_c_warning[] =
 +"only found copies from modified paths due to too many files.";
 +
 +static const char rename_limit_advice[] =
 +"you may want to set your %s variable to at least "
 +"%d and retry the command.";
 +
 +void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
 +{
 +      if (degraded_cc)
 +              warning(degrade_cc_to_c_warning);
 +      else if (needed)
 +              warning(rename_limit_warning);
 +      else
 +              return;
 +      if (0 < needed && needed < 32767)
 +              warning(rename_limit_advice, varname, needed);
 +}
 +
  void diff_flush(struct diff_options *options)
  {
        struct diff_queue_struct *q = &diff_queued_diff;
        int i, output_format = options->output_format;
        int separator = 0;
 +      int dirstat_by_line = 0;
  
        /*
         * Order: raw, stat, summary, patch
                separator++;
        }
  
 -      if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT)) {
 +      if (output_format & DIFF_FORMAT_DIRSTAT && DIFF_OPT_TST(options, DIRSTAT_BY_LINE))
 +              dirstat_by_line = 1;
 +
 +      if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT) ||
 +          dirstat_by_line) {
                struct diffstat_t diffstat;
  
                memset(&diffstat, 0, sizeof(struct diffstat_t));
                        show_stats(&diffstat, options);
                if (output_format & DIFF_FORMAT_SHORTSTAT)
                        show_shortstats(&diffstat, options);
 +              if (output_format & DIFF_FORMAT_DIRSTAT)
 +                      show_dirstat_by_line(&diffstat, options);
                free_diffstat_info(&diffstat);
                separator++;
        }
 -      if (output_format & DIFF_FORMAT_DIRSTAT)
 +      if ((output_format & DIFF_FORMAT_DIRSTAT) && !dirstat_by_line)
                show_dirstat(options);
  
        if (output_format & DIFF_FORMAT_SUMMARY && !is_summary_empty(q)) {
@@@ -4432,10 -4236,6 +4440,10 @@@ void diffcore_std(struct diff_options *
  int diff_result_code(struct diff_options *opt, int status)
  {
        int result = 0;
 +
 +      diff_warn_rename_limit("diff.renamelimit",
 +                             opt->needed_rename_limit,
 +                             opt->degraded_cc_to_c);
        if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
            !(opt->output_format & DIFF_FORMAT_CHECKDIFF))
                return status;
@@@ -4549,20 -4349,20 +4557,20 @@@ void diff_change(struct diff_options *o
                DIFF_OPT_SET(options, HAS_CHANGES);
  }
  
 -void diff_unmerge(struct diff_options *options,
 -                const char *path,
 -                unsigned mode, const unsigned char *sha1)
 +struct diff_filepair *diff_unmerge(struct diff_options *options, const char *path)
  {
 +      struct diff_filepair *pair;
        struct diff_filespec *one, *two;
  
        if (options->prefix &&
            strncmp(path, options->prefix, options->prefix_length))
 -              return;
 +              return NULL;
  
        one = alloc_filespec(path);
        two = alloc_filespec(path);
 -      fill_filespec(one, sha1, mode);
 -      diff_queue(&diff_queued_diff, one, two)->is_unmerged = 1;
 +      pair = diff_queue(&diff_queued_diff, one, two);
 +      pair->is_unmerged = 1;
 +      return pair;
  }
  
  static char *run_textconv(const char *pgm, struct diff_filespec *spec,