Do not use memcmp(sha1_1, sha1_2, 20) with hardcoded length.

rientjes · Junio C Hamano · commit a89fccd28197 · 2006-08-17T14:23:53.000-07:00
Introduces global inline:

	hashcmp(const unsigned char *sha1, const unsigned char *sha2)

Uses memcmp for comparison and returns the result based on the length of
the hash name (a future runtime decision).

Acked-by: Alex Riesen &lt;raa.lkml@gmail.com&gt;
Signed-off-by: David Rientjes &lt;rientjes@google.com&gt;
Signed-off-by: Junio C Hamano &lt;junkio@cox.net&gt;
diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
@@ -69,7 +69,7 @@ static int new_parent(int idx)
 	int i;
 	unsigned char *sha1 = parent_sha1[idx];
 	for (i = 0; i < idx; i++) {
-		if (!memcmp(parent_sha1[i], sha1, 20)) {
+		if (!hashcmp(parent_sha1[i], sha1)) {
 			error("duplicate parent %s ignored", sha1_to_hex(sha1));
 			return 0;
 		}
diff --git a/builtin-diff-stages.c b/builtin-diff-stages.c
@@ -46,7 +46,7 @@ static void diff_stages(int stage1, int stage2, const char **pathspec)
 		else if (!two)
 			diff_addremove(&diff_options, '-', ntohl(one->ce_mode),
 				       one->sha1, name, NULL);
-		else if (memcmp(one->sha1, two->sha1, 20) ||
+		else if (hashcmp(one->sha1, two->sha1) ||
 			 (one->ce_mode != two->ce_mode) ||
 			 diff_options.find_copies_harder)
 			diff_change(&diff_options,
diff --git a/builtin-diff.c b/builtin-diff.c
@@ -69,7 +69,7 @@ static void stuff_change(struct diff_options *opt,
 	struct diff_filespec *one, *two;
 
 	if (!is_null_sha1(old_sha1) && !is_null_sha1(new_sha1) &&
-	    !memcmp(old_sha1, new_sha1, 20))
+	    !hashcmp(old_sha1, new_sha1))
 		return;
 
 	if (opt->reverse_diff) {
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
@@ -441,7 +441,7 @@ static int locate_object_entry_hash(const unsigned char *sha1)
 	memcpy(&ui, sha1, sizeof(unsigned int));
 	i = ui % object_ix_hashsz;
 	while (0 < object_ix[i]) {
-		if (!memcmp(sha1, objects[object_ix[i]-1].sha1, 20))
+		if (!hashcmp(sha1, objects[object_ix[i] - 1].sha1))
 			return i;
 		if (++i == object_ix_hashsz)
 			i = 0;
@@ -607,7 +607,7 @@ static struct pbase_tree_cache *pbase_tree_get(const unsigned char *sha1)
 	 */
 	for (neigh = 0; neigh < 8; neigh++) {
 		ent = pbase_tree_cache[my_ix];
-		if (ent && !memcmp(ent->sha1, sha1, 20)) {
+		if (ent && !hashcmp(ent->sha1, sha1)) {
 			ent->ref++;
 			return ent;
 		}
@@ -789,7 +789,7 @@ static void add_preferred_base(unsigned char *sha1)
 		return;
 
 	for (it = pbase_tree; it; it = it->next) {
-		if (!memcmp(it->pcache.sha1, tree_sha1, 20)) {
+		if (!hashcmp(it->pcache.sha1, tree_sha1)) {
 			free(data);
 			return;
 		}
@@ -931,7 +931,7 @@ static struct object_entry **create_sorted_list(entry_sort_t sort)
 
 static int sha1_sort(const struct object_entry *a, const struct object_entry *b)
 {
-	return memcmp(a->sha1, b->sha1, 20);
+	return hashcmp(a->sha1, b->sha1);
 }
 
 static struct object_entry **create_final_object_list(void)
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
@@ -378,7 +378,7 @@ static int append_head_ref(const char *refname, const unsigned char *sha1)
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
 	 */
-	if (get_sha1(refname + ofs, tmp) || memcmp(tmp, sha1, 20))
+	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
 		ofs = 5;
 	return append_ref(refname + ofs, sha1);
 }
@@ -442,7 +442,7 @@ static int rev_is_head(char *head_path, int headlen, char *name,
 {
 	int namelen;
 	if ((!head_path[0]) ||
-	    (head_sha1 && sha1 && memcmp(head_sha1, sha1, 20)))
+	    (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
 		return 0;
 	namelen = strlen(name);
 	if ((headlen < namelen) ||
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
@@ -136,7 +136,7 @@ static void added_object(unsigned char *sha1, const char *type, void *data, unsi
 	struct delta_info *info;
 
 	while ((info = *p) != NULL) {
-		if (!memcmp(info->base_sha1, sha1, 20)) {
+		if (!hashcmp(info->base_sha1, sha1)) {
 			*p = info->next;
 			p = &delta_list;
 			resolve_delta(type, data, size, info->delta, info->size);
@@ -292,7 +292,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)
 	unpack_all();
 	SHA1_Update(&ctx, buffer, offset);
 	SHA1_Final(sha1, &ctx);
-	if (memcmp(fill(20), sha1, 20))
+	if (hashcmp(fill(20), sha1))
 		die("final sha1 did not match");
 	use(20);
 
diff --git a/builtin-update-index.c b/builtin-update-index.c
@@ -378,7 +378,7 @@ static int unresolve_one(const char *path)
 		ret = -1;
 		goto free_return;
 	}
-	if (!memcmp(ce_2->sha1, ce_3->sha1, 20) &&
+	if (!hashcmp(ce_2->sha1, ce_3->sha1) &&
 	    ce_2->ce_mode == ce_3->ce_mode) {
 		fprintf(stderr, "%s: identical in both, skipping.\n",
 			path);
@@ -460,7 +460,7 @@ static int do_reupdate(int ac, const char **av,
 			old = read_one_ent(NULL, head_sha1,
 					   ce->name, ce_namelen(ce), 0);
 		if (old && ce->ce_mode == old->ce_mode &&
-		    !memcmp(ce->sha1, old->sha1, 20)) {
+		    !hashcmp(ce->sha1, old->sha1)) {
 			free(old);
 			continue; /* unchanged */
 		}
diff --git a/cache.h b/cache.h
@@ -214,6 +214,10 @@ static inline int is_null_sha1(const unsigned char *sha1)
 {
 	return !memcmp(sha1, null_sha1, 20);
 }
+static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
+{
+	return memcmp(sha1, sha2, 20);
+}
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
diff --git a/combine-diff.c b/combine-diff.c
@@ -688,8 +688,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	for (i = 0; i < num_parent; i++) {
 		int j;
 		for (j = 0; j < i; j++) {
-			if (!memcmp(elem->parent[i].sha1,
-				    elem->parent[j].sha1, 20)) {
+			if (!hashcmp(elem->parent[i].sha1,
+				     elem->parent[j].sha1)) {
 				reuse_combine_diff(sline, cnt, i, j);
 				break;
 			}
diff --git a/commit.c b/commit.c
@@ -123,7 +123,7 @@ static int commit_graft_pos(const unsigned char *sha1)
 	while (lo < hi) {
 		int mi = (lo + hi) / 2;
 		struct commit_graft *graft = commit_graft[mi];
-		int cmp = memcmp(sha1, graft->sha1, 20);
+		int cmp = hashcmp(sha1, graft->sha1);
 		if (!cmp)
 			return mi;
 		if (cmp < 0)
diff --git a/convert-objects.c b/convert-objects.c
@@ -39,7 +39,7 @@ static struct entry *lookup_entry(unsigned char *sha1)
 	while (low < high) {
 		int next = (low + high) / 2;
 		struct entry *n = convert[next];
-		int cmp = memcmp(sha1, n->old_sha1, 20);
+		int cmp = hashcmp(sha1, n->old_sha1);
 		if (!cmp)
 			return n;
 		if (cmp < 0) {
diff --git a/diff-lib.c b/diff-lib.c
@@ -215,7 +215,7 @@ static int show_modified(struct rev_info *revs,
 	}
 
 	oldmode = old->ce_mode;
-	if (mode == oldmode && !memcmp(sha1, old->sha1, 20) &&
+	if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
 	    !revs->diffopt.find_copies_harder)
 		return 0;
 
diff --git a/diff.c b/diff.c
@@ -1140,7 +1140,7 @@ static int work_tree_matches(const char *name, const unsigned char *sha1)
 	if ((lstat(name, &st) < 0) ||
 	    !S_ISREG(st.st_mode) || /* careful! */
 	    ce_match_stat(ce, &st, 0) ||
-	    memcmp(sha1, ce->sha1, 20))
+	    hashcmp(sha1, ce->sha1))
 		return 0;
 	/* we return 1 only when we can stat, it is a regular file,
 	 * stat information matches, and sha1 recorded in the cache
@@ -1168,7 +1168,7 @@ static struct sha1_size_cache *locate_size_cache(unsigned char *sha1,
 	while (last > first) {
 		int cmp, next = (last + first) >> 1;
 		e = sha1_size_cache[next];
-		cmp = memcmp(e->sha1, sha1, 20);
+		cmp = hashcmp(e->sha1, sha1);
 		if (!cmp)
 			return e;
 		if (cmp < 0) {
@@ -1579,7 +1579,7 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 		;
 	}
 
-	if (memcmp(one->sha1, two->sha1, 20)) {
+	if (hashcmp(one->sha1, two->sha1)) {
 		int abbrev = o->full_index ? 40 : DEFAULT_ABBREV;
 
 		len += snprintf(msg + len, sizeof(msg) - len,
@@ -2098,7 +2098,7 @@ int diff_unmodified_pair(struct diff_filepair *p)
 	 * dealing with a change.
 	 */
 	if (one->sha1_valid && two->sha1_valid &&
-	    !memcmp(one->sha1, two->sha1, sizeof(one->sha1)))
+	    !hashcmp(one->sha1, two->sha1))
 		return 1; /* no change */
 	if (!one->sha1_valid && !two->sha1_valid)
 		return 1; /* both look at the same file on the filesystem. */
@@ -2237,7 +2237,7 @@ static void diff_resolve_rename_copy(void)
 			if (!p->status)
 				p->status = DIFF_STATUS_RENAMED;
 		}
-		else if (memcmp(p->one->sha1, p->two->sha1, 20) ||
+		else if (hashcmp(p->one->sha1, p->two->sha1) ||
 			 p->one->mode != p->two->mode)
 			p->status = DIFF_STATUS_MODIFIED;
 		else {
diff --git a/diffcore-break.c b/diffcore-break.c
@@ -56,7 +56,7 @@ static int should_break(struct diff_filespec *src,
 		return 0; /* leave symlink rename alone */
 
 	if (src->sha1_valid && dst->sha1_valid &&
-	    !memcmp(src->sha1, dst->sha1, 20))
+	    !hashcmp(src->sha1, dst->sha1))
 		return 0; /* they are the same */
 
 	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
diff --git a/diffcore-rename.c b/diffcore-rename.c
@@ -101,7 +101,7 @@ static int is_exact_match(struct diff_filespec *src,
 			  int contents_too)
 {
 	if (src->sha1_valid && dst->sha1_valid &&
-	    !memcmp(src->sha1, dst->sha1, 20))
+	    !hashcmp(src->sha1, dst->sha1))
 		return 1;
 	if (!contents_too)
 		return 0;
diff --git a/dump-cache-tree.c b/dump-cache-tree.c
@@ -33,7 +33,7 @@ static int dump_cache_tree(struct cache_tree *it,
 	}
 	else {
 		dump_one(it, pfx, "");
-		if (memcmp(it->sha1, ref->sha1, 20) ||
+		if (hashcmp(it->sha1, ref->sha1) ||
 		    ref->entry_count != it->entry_count ||
 		    ref->subtree_nr != it->subtree_nr) {
 			dump_one(ref, pfx, "#(ref) ");
diff --git a/http-fetch.c b/http-fetch.c
@@ -301,7 +301,7 @@ static void finish_object_request(struct object_request *obj_req)
 		unlink(obj_req->tmpfile);
 		return;
 	}
-	if (memcmp(obj_req->sha1, obj_req->real_sha1, 20)) {
+	if (hashcmp(obj_req->sha1, obj_req->real_sha1)) {
 		unlink(obj_req->tmpfile);
 		return;
 	}
@@ -1070,7 +1070,7 @@ static int fetch_object(struct alt_base *repo, unsigned char *sha1)
 	int ret = 0;
 	struct object_request *obj_req = object_queue_head;
 
-	while (obj_req != NULL && memcmp(obj_req->sha1, sha1, 20))
+	while (obj_req != NULL && hashcmp(obj_req->sha1, sha1))
 		obj_req = obj_req->next;
 	if (obj_req == NULL)
 		return error("Couldn't find request for %s in the queue", hex);
@@ -1109,7 +1109,7 @@ static int fetch_object(struct alt_base *repo, unsigned char *sha1)
 	} else if (obj_req->zret != Z_STREAM_END) {
 		corrupt_object_found++;
 		ret = error("File %s (%s) corrupt", hex, obj_req->url);
-	} else if (memcmp(obj_req->sha1, obj_req->real_sha1, 20)) {
+	} else if (hashcmp(obj_req->sha1, obj_req->real_sha1)) {
 		ret = error("File %s has bad hash", hex);
 	} else if (obj_req->rename < 0) {
 		ret = error("unable to write sha1 filename %s",
diff --git a/http-push.c b/http-push.c
@@ -745,7 +745,7 @@ static void finish_request(struct transfer_request *request)
 			SHA1_Final(request->real_sha1, &request->c);
 			if (request->zret != Z_STREAM_END) {
 				unlink(request->tmpfile);
-			} else if (memcmp(request->obj->sha1, request->real_sha1, 20)) {
+			} else if (hashcmp(request->obj->sha1, request->real_sha1)) {
 				unlink(request->tmpfile);
 			} else {
 				request->rename =
@@ -2416,7 +2416,7 @@ int main(int argc, char **argv)
 
 		if (!ref->peer_ref)
 			continue;
-		if (!memcmp(ref->old_sha1, ref->peer_ref->new_sha1, 20)) {
+		if (!hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
 			if (push_verbosely || 1)
 				fprintf(stderr, "'%s': up-to-date\n", ref->name);
 			continue;
diff --git a/index-pack.c b/index-pack.c
@@ -82,7 +82,7 @@ static void parse_pack_header(void)
 	SHA1_Init(&ctx);
 	SHA1_Update(&ctx, pack_base, pack_size - 20);
 	SHA1_Final(sha1, &ctx);
-	if (memcmp(sha1, pack_base + pack_size - 20, 20))
+	if (hashcmp(sha1, pack_base + pack_size - 20))
 		die("packfile '%s' SHA1 mismatch", pack_name);
 }
 
@@ -189,7 +189,7 @@ static int find_delta(const unsigned char *base_sha1)
                 struct delta_entry *delta = &deltas[next];
                 int cmp;
 
-                cmp = memcmp(base_sha1, delta->base_sha1, 20);
+                cmp = hashcmp(base_sha1, delta->base_sha1);
                 if (!cmp)
                         return next;
                 if (cmp < 0) {
@@ -210,9 +210,9 @@ static int find_deltas_based_on_sha1(const unsigned char *base_sha1,
 
 	if (first < 0)
 		return -1;
-	while (first > 0 && !memcmp(deltas[first-1].base_sha1, base_sha1, 20))
+	while (first > 0 && !hashcmp(deltas[first - 1].base_sha1, base_sha1))
 		--first;
-	while (last < end && !memcmp(deltas[last+1].base_sha1, base_sha1, 20))
+	while (last < end && !hashcmp(deltas[last + 1].base_sha1, base_sha1))
 		++last;
 	*first_index = first;
 	*last_index = last;
@@ -278,7 +278,7 @@ static int compare_delta_entry(const void *a, const void *b)
 {
 	const struct delta_entry *delta_a = a;
 	const struct delta_entry *delta_b = b;
-	return memcmp(delta_a->base_sha1, delta_b->base_sha1, 20);
+	return hashcmp(delta_a->base_sha1, delta_b->base_sha1);
 }
 
 static void parse_pack_objects(void)
@@ -350,7 +350,7 @@ static int sha1_compare(const void *_a, const void *_b)
 {
 	struct object_entry *a = *(struct object_entry **)_a;
 	struct object_entry *b = *(struct object_entry **)_b;
-	return memcmp(a->sha1, b->sha1, 20);
+	return hashcmp(a->sha1, b->sha1);
 }
 
 static void write_index_file(const char *index_name, unsigned char *sha1)
diff --git a/merge-tree.c b/merge-tree.c
@@ -152,7 +152,7 @@ static int same_entry(struct name_entry *a, struct name_entry *b)
 {
 	return	a->sha1 &&
 		b->sha1 &&
-		!memcmp(a->sha1, b->sha1, 20) &&
+		!hashcmp(a->sha1, b->sha1) &&
 		a->mode == b->mode;
 }
 
diff --git a/object.c b/object.c
@@ -58,7 +58,7 @@ struct object *lookup_object(const unsigned char *sha1)
 
 	i = hashtable_index(sha1);
 	while ((obj = obj_hash[i]) != NULL) {
-		if (!memcmp(sha1, obj->sha1, 20))
+		if (!hashcmp(sha1, obj->sha1))
 			break;
 		i++;
 		if (i == obj_hash_size)
diff --git a/pack-check.c b/pack-check.c
@@ -29,10 +29,10 @@ static int verify_packfile(struct packed_git *p)
 	pack_base = p->pack_base;
 	SHA1_Update(&ctx, pack_base, pack_size - 20);
 	SHA1_Final(sha1, &ctx);
-	if (memcmp(sha1, (char *) pack_base + pack_size - 20, 20))
+	if (hashcmp(sha1, (unsigned char *)pack_base + pack_size - 20))
 		return error("Packfile %s SHA1 mismatch with itself",
 			     p->pack_name);
-	if (memcmp(sha1, (char *) index_base + index_size - 40, 20))
+	if (hashcmp(sha1, (unsigned char *)index_base + index_size - 40))
 		return error("Packfile %s SHA1 mismatch with idx",
 			     p->pack_name);
 
@@ -135,7 +135,7 @@ int verify_pack(struct packed_git *p, int verbose)
 	SHA1_Init(&ctx);
 	SHA1_Update(&ctx, index_base, index_size - 20);
 	SHA1_Final(sha1, &ctx);
-	if (memcmp(sha1, (char *) index_base + index_size - 20, 20))
+	if (hashcmp(sha1, (unsigned char *)index_base + index_size - 20))
 		ret = error("Packfile index for %s SHA1 mismatch",
 			    p->pack_name);
 
diff --git a/pack-redundant.c b/pack-redundant.c
diff --git a/read-cache.c b/read-cache.c
diff --git a/refs.c b/refs.c
diff --git a/send-pack.c b/send-pack.c
diff --git a/sha1_file.c b/sha1_file.c
diff --git a/sha1_name.c b/sha1_name.c
diff --git a/ssh-fetch.c b/ssh-fetch.c
diff --git a/tree-diff.c b/tree-diff.c
diff --git a/unpack-trees.c b/unpack-trees.c

Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ static int new_parent(int idx)`
`69`	`69`	`int i;`
`70`	`70`	`unsigned char *sha1 = parent_sha1[idx];`
`71`	`71`	`for (i = 0; i < idx; i++) {`
`72`		`- if (!memcmp(parent_sha1[i], sha1, 20)) {`
	`72`	`+ if (!hashcmp(parent_sha1[i], sha1)) {`
`73`	`73`	`error("duplicate parent %s ignored", sha1_to_hex(sha1));`
`74`	`74`	`return 0;`
`75`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -214,6 +214,10 @@ static inline int is_null_sha1(const unsigned char *sha1)`
`214`	`214`	`{`
`215`	`215`	`return !memcmp(sha1, null_sha1, 20);`
`216`	`216`	`}`
	`217`	`+static inline int hashcmp(const unsigned char sha1, const unsigned char sha2)`
	`218`	`+{`
	`219`	`+ return memcmp(sha1, sha2, 20);`
	`220`	`+}`
`217`	`221`
`218`	`222`	`int git_mkstemp(char path, size_t n, const char template);`
`219`	`223`