From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 10 Mar 2015 20:52:38 +0000 (-0700)
Subject: Merge branch 'jk/diffcore-rename-duplicate'
X-Git-Tag: v2.4.0-rc0~52
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/2d659f7d6e6071701a4f532487edf139b3e1fa9d?ds=inline;hp=-c

Merge branch 'jk/diffcore-rename-duplicate'

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
---

2d659f7d6e6071701a4f532487edf139b3e1fa9d
diff --combined diffcore-rename.c
index 4e132f1fdb,361eed9fbc..af1fe08861
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@@ -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;
  
@@@ -27,16 -26,33 +26,33 @@@
  		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,
@@@ -259,14 -275,15 +275,14 @@@
  	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))
@@@ -582,8 -603,7 +602,7 @@@
  			 * 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;
@@@ -613,8 -633,7 +632,7 @@@
  			 */
  			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;