Merge branch 'jk/diffcore-rename-duplicate'
authorJunio C Hamano <gitster@pobox.com>
Tue, 10 Mar 2015 20:52:38 +0000 (13:52 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 10 Mar 2015 20:52:39 +0000 (13:52 -0700)
A corrupt input to "git diff -M" can cause us to segfault.

* jk/diffcore-rename-duplicate:
diffcore-rename: avoid processing duplicate destinations
diffcore-rename: split locate_rename_dst into two functions

1  2 
diffcore-rename.c
diff --combined diffcore-rename.c
index 4e132f1fdb68ed3c930ca8224a5403c9b406cc3b,361eed9fbcaa282e4e0ac14f2ac28a0b5d8b2826..af1fe08861e6862985ac8fb9311b17568cb9e547
@@@ -15,8 -15,7 +15,7 @@@ static struct diff_rename_dst 
  } *rename_dst;
  static int rename_dst_nr, rename_dst_alloc;
  
- static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
-                                                int insert_ok)
+ static int find_rename_dst(struct diff_filespec *two)
  {
        int first, last;
  
                struct diff_rename_dst *dst = &(rename_dst[next]);
                int cmp = strcmp(two->path, dst->two->path);
                if (!cmp)
-                       return dst;
+                       return next;
                if (cmp < 0) {
                        last = next;
                        continue;
                }
                first = next+1;
        }
-       /* not found */
-       if (!insert_ok)
-               return NULL;
+       return -first - 1;
+ }
+ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two)
+ {
+       int ofs = find_rename_dst(two);
+       return ofs < 0 ? NULL : &rename_dst[ofs];
+ }
+ /*
+  * Returns 0 on success, -1 if we found a duplicate.
+  */
+ static int add_rename_dst(struct diff_filespec *two)
+ {
+       int first = find_rename_dst(two);
+       if (first >= 0)
+               return -1;
+       first = -first - 1;
        /* insert to make it at "first" */
        ALLOC_GROW(rename_dst, rename_dst_nr + 1, rename_dst_alloc);
        rename_dst_nr++;
@@@ -46,7 -62,7 +62,7 @@@
        rename_dst[first].two = alloc_filespec(two->path);
        fill_filespec(rename_dst[first].two, two->sha1, two->sha1_valid, two->mode);
        rename_dst[first].pair = NULL;
-       return &(rename_dst[first]);
+       return 0;
  }
  
  /* Table of rename/copy src files */
@@@ -147,11 -163,9 +163,11 @@@ static int estimate_similarity(struct d
         * is a possible size - we really should have a flag to
         * say whether the size is valid or not!)
         */
 -      if (!src->cnt_data && diff_populate_filespec(src, 1))
 +      if (!src->cnt_data &&
 +          diff_populate_filespec(src, CHECK_SIZE_ONLY))
                return 0;
 -      if (!dst->cnt_data && diff_populate_filespec(dst, 1))
 +      if (!dst->cnt_data &&
 +          diff_populate_filespec(dst, CHECK_SIZE_ONLY))
                return 0;
  
        max_size = ((src->size > dst->size) ? src->size : dst->size);
@@@ -244,12 -258,14 +260,12 @@@ struct file_similarity 
  
  static unsigned int hash_filespec(struct diff_filespec *filespec)
  {
 -      unsigned int hash;
        if (!filespec->sha1_valid) {
                if (diff_populate_filespec(filespec, 0))
                        return 0;
                hash_sha1_file(filespec->data, filespec->size, "blob", filespec->sha1);
        }
 -      memcpy(&hash, filespec->sha1, sizeof(hash));
 -      return hash;
 +      return sha1hash(filespec->sha1);
  }
  
  static int find_identical_files(struct hashmap *srcs,
        int renames = 0;
  
        struct diff_filespec *target = rename_dst[dst_index].two;
 -      struct file_similarity *p, *best, dst;
 +      struct file_similarity *p, *best = NULL;
        int i = 100, best_score = -1;
  
        /*
         * Find the best source match for specified destination.
         */
 -      best = NULL;
 -      hashmap_entry_init(&dst, hash_filespec(target));
 -      for (p = hashmap_get(srcs, &dst, NULL); p; p = hashmap_get_next(srcs, p)) {
 +      p = hashmap_get_from_hash(srcs, hash_filespec(target), NULL);
 +      for (; p; p = hashmap_get_next(srcs, p)) {
                int score;
                struct diff_filespec *source = p->filespec;
  
@@@ -450,8 -467,12 +466,12 @@@ void diffcore_rename(struct diff_option
                        else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
                                 is_empty_blob_sha1(p->two->sha1))
                                continue;
-                       else
-                               locate_rename_dst(p->two, 1);
+                       else if (add_rename_dst(p->two) < 0) {
+                               warning("skipping rename detection, detected"
+                                       " duplicate destination '%s'",
+                                       p->two->path);
+                               goto cleanup;
+                       }
                }
                else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
                         is_empty_blob_sha1(p->one->sha1))
                         * We would output this create record if it has
                         * not been turned into a rename/copy already.
                         */
-                       struct diff_rename_dst *dst =
-                               locate_rename_dst(p->two, 0);
+                       struct diff_rename_dst *dst = locate_rename_dst(p->two);
                        if (dst && dst->pair) {
                                diff_q(&outq, dst->pair);
                                pair_to_free = p;
                         */
                        if (DIFF_PAIR_BROKEN(p)) {
                                /* broken delete */
-                               struct diff_rename_dst *dst =
-                                       locate_rename_dst(p->one, 0);
+                               struct diff_rename_dst *dst = locate_rename_dst(p->one);
                                if (dst && dst->pair)
                                        /* counterpart is now rename/copy */
                                        pair_to_free = p;