Merge branch 'jk/use-oid-pos'

Code clean-up to ensure our use of hashtables using object names as
keys use the "struct object_id" objects, not the raw hash values.

* jk/use-oid-pos:
  oid_pos(): access table through const pointers
  hash_pos(): convert to oid_pos()
  rerere: use strmap to store rerere directories
  rerere: tighten rr-cache dirname check
  rerere: check dirname format while iterating rr_cache directory
  commit_graft_pos(): take an oid instead of a bare hash

builtin/name-rev.c

@@ -390,10 +390,10 @@ static void name_tips(void)
 	}
 }
 
-static const unsigned char *nth_tip_table_ent(size_t ix, void *table_)
+static const struct object_id *nth_tip_table_ent(size_t ix, const void *table_)
 {
-	struct tip_table_entry *table = table_;
+	const struct tip_table_entry *table = table_;
-	return table[ix].oid.hash;
+	return &table[ix].oid;
 }
 
 static const char *get_exact_ref_match(const struct object *o)
@@ -408,8 +408,8 @@ static const char *get_exact_ref_match(const struct object *o)
 		tip_table.sorted = 1;
 	}
 
-	found = hash_pos(o->oid.hash, tip_table.table, tip_table.nr,
+	found = oid_pos(&o->oid, tip_table.table, tip_table.nr,
-			 nth_tip_table_ent);
+			nth_tip_table_ent);
 	if (0 <= found)
 		return tip_table.table[found].refname;
 	return NULL;

commit-graph.c

@@ -1012,10 +1012,10 @@ static int write_graph_chunk_oids(struct hashfile *f,
 	return 0;
 }
 
-static const unsigned char *commit_to_sha1(size_t index, void *table)
+static const struct object_id *commit_to_oid(size_t index, const void *table)
 {
-	struct commit **commits = table;
+	const struct commit * const *commits = table;
-	return commits[index]->object.oid.hash;
+	return &commits[index]->object.oid;
 }
 
 static int write_graph_chunk_data(struct hashfile *f,
@@ -1043,10 +1043,10 @@ static int write_graph_chunk_data(struct hashfile *f,
 		if (!parent)
 			edge_value = GRAPH_PARENT_NONE;
 		else {
-			edge_value = hash_pos(parent->item->object.oid.hash,
+			edge_value = oid_pos(&parent->item->object.oid,
-					      ctx->commits.list,
+					     ctx->commits.list,
-					      ctx->commits.nr,
+					     ctx->commits.nr,
-					      commit_to_sha1);
+					     commit_to_oid);
 
 			if (edge_value >= 0)
 				edge_value += ctx->new_num_commits_in_base;
@@ -1074,10 +1074,10 @@ static int write_graph_chunk_data(struct hashfile *f,
 		else if (parent->next)
 			edge_value = GRAPH_EXTRA_EDGES_NEEDED | num_extra_edges;
 		else {
-			edge_value = hash_pos(parent->item->object.oid.hash,
+			edge_value = oid_pos(&parent->item->object.oid,
-					      ctx->commits.list,
+					     ctx->commits.list,
-					      ctx->commits.nr,
+					     ctx->commits.nr,
-					      commit_to_sha1);
+					     commit_to_oid);
 
 			if (edge_value >= 0)
 				edge_value += ctx->new_num_commits_in_base;
@@ -1143,10 +1143,10 @@ static int write_graph_chunk_extra_edges(struct hashfile *f,
 
 		/* Since num_parents > 2, this initializer is safe. */
 		for (parent = (*list)->parents->next; parent; parent = parent->next) {
-			int edge_value = hash_pos(parent->item->object.oid.hash,
+			int edge_value = oid_pos(&parent->item->object.oid,
-						  ctx->commits.list,
+						 ctx->commits.list,
-						  ctx->commits.nr,
+						 ctx->commits.nr,
-						  commit_to_sha1);
+						 commit_to_oid);
 
 			if (edge_value >= 0)
 				edge_value += ctx->new_num_commits_in_base;

commit.c

@@ -105,23 +105,23 @@ static timestamp_t parse_commit_date(const char *buf, const char *tail)
 	return parse_timestamp(dateptr, NULL, 10);
 }
 
-static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
+static const struct object_id *commit_graft_oid_access(size_t index, const void *table)
 {
-	struct commit_graft **commit_graft_table = table;
+	const struct commit_graft * const *commit_graft_table = table;
-	return commit_graft_table[index]->oid.hash;
+	return &commit_graft_table[index]->oid;
 }
 
-int commit_graft_pos(struct repository *r, const unsigned char *sha1)
+int commit_graft_pos(struct repository *r, const struct object_id *oid)
 {
-	return hash_pos(sha1, r->parsed_objects->grafts,
+	return oid_pos(oid, r->parsed_objects->grafts,
-			r->parsed_objects->grafts_nr,
+		       r->parsed_objects->grafts_nr,
-			commit_graft_sha1_access);
+		       commit_graft_oid_access);
 }
 
 int register_commit_graft(struct repository *r, struct commit_graft *graft,
 			  int ignore_dups)
 {
-	int pos = commit_graft_pos(r, graft->oid.hash);
+	int pos = commit_graft_pos(r, &graft->oid);
 
 	if (0 <= pos) {
 		if (ignore_dups)
@@ -232,7 +232,7 @@ struct commit_graft *lookup_commit_graft(struct repository *r, const struct obje
 {
 	int pos;
 	prepare_commit_graft(r);
-	pos = commit_graft_pos(r, oid->hash);
+	pos = commit_graft_pos(r, oid);
 	if (pos < 0)
 		return NULL;
 	return r->parsed_objects->grafts[pos];

commit.h

@@ -239,7 +239,7 @@ typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
 struct commit_graft *read_graft_line(struct strbuf *line);
 /* commit_graft_pos returns an index into r->parsed_objects->grafts. */
-int commit_graft_pos(struct repository *r, const unsigned char *sha1);
+int commit_graft_pos(struct repository *r, const struct object_id *oid);
 int register_commit_graft(struct repository *r, struct commit_graft *, int);
 void prepare_commit_graft(struct repository *r);
 struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);

hash-lookup.c

@@ -1,9 +1,9 @@
 #include "cache.h"
 #include "hash-lookup.h"
 
-static uint32_t take2(const unsigned char *hash)
+static uint32_t take2(const struct object_id *oid, size_t ofs)
 {
-	return ((hash[0] << 8) | hash[1]);
+	return ((oid->hash[ofs] << 8) | oid->hash[ofs + 1]);
 }
 
 /*
@@ -47,11 +47,11 @@ static uint32_t take2(const unsigned char *hash)
  */
 /*
  * The table should contain "nr" elements.
- * The hash of element i (between 0 and nr - 1) should be returned
+ * The oid of element i (between 0 and nr - 1) should be returned
  * by "fn(i, table)".
  */
-int hash_pos(const unsigned char *hash, void *table, size_t nr,
+int oid_pos(const struct object_id *oid, const void *table, size_t nr,
-	     hash_access_fn fn)
+	    oid_access_fn fn)
 {
 	size_t hi = nr;
 	size_t lo = 0;
@@ -64,9 +64,9 @@ int hash_pos(const unsigned char *hash, void *table, size_t nr,
 		size_t lov, hiv, miv, ofs;
 
 		for (ofs = 0; ofs < the_hash_algo->rawsz - 2; ofs += 2) {
-			lov = take2(fn(0, table) + ofs);
+			lov = take2(fn(0, table), ofs);
-			hiv = take2(fn(nr - 1, table) + ofs);
+			hiv = take2(fn(nr - 1, table), ofs);
-			miv = take2(hash + ofs);
+			miv = take2(oid, ofs);
 			if (miv < lov)
 				return -1;
 			if (hiv < miv)
@@ -88,7 +88,7 @@ int hash_pos(const unsigned char *hash, void *table, size_t nr,
 
 	do {
 		int cmp;
-		cmp = hashcmp(fn(mi, table), hash);
+		cmp = oidcmp(fn(mi, table), oid);
 		if (!cmp)
 			return mi;
 		if (cmp > 0)

hash-lookup.h

@@ -1,12 +1,12 @@
 #ifndef HASH_LOOKUP_H
 #define HASH_LOOKUP_H
 
-typedef const unsigned char *hash_access_fn(size_t index, void *table);
+typedef const struct object_id *oid_access_fn(size_t index, const void *table);
 
-int hash_pos(const unsigned char *hash,
+int oid_pos(const struct object_id *oid,
-	     void *table,
+	    const void *table,
-	     size_t nr,
+	    size_t nr,
-	     hash_access_fn fn);
+	    oid_access_fn fn);
 
 /*
  * Searches for hash in table, using the given fanout table to determine the

oid-array.c

@@ -22,16 +22,16 @@ void oid_array_sort(struct oid_array *array)
 	array->sorted = 1;
 }
 
-static const unsigned char *sha1_access(size_t index, void *table)
+static const struct object_id *oid_access(size_t index, const void *table)
 {
-	struct object_id *array = table;
+	const struct object_id *array = table;
-	return array[index].hash;
+	return &array[index];
 }
 
 int oid_array_lookup(struct oid_array *array, const struct object_id *oid)
 {
 	oid_array_sort(array);
-	return hash_pos(oid->hash, array->oid, array->nr, sha1_access);
+	return oid_pos(oid, array->oid, array->nr, oid_access);
 }
 
 void oid_array_clear(struct oid_array *array)

pack-bitmap-write.c

@@ -610,10 +610,10 @@ static inline void dump_bitmap(struct hashfile *f, struct ewah_bitmap *bitmap)
 		die("Failed to write bitmap index");
 }
 
-static const unsigned char *sha1_access(size_t pos, void *table)
+static const struct object_id *oid_access(size_t pos, const void *table)
 {
-	struct pack_idx_entry **index = table;
+	const struct pack_idx_entry * const *index = table;
-	return index[pos]->oid.hash;
+	return &index[pos]->oid;
 }
 
 static void write_selected_commits_v1(struct hashfile *f,
@@ -626,7 +626,7 @@ static void write_selected_commits_v1(struct hashfile *f,
 		struct bitmapped_commit *stored = &writer.selected[i];
 
 		int commit_pos =
-			hash_pos(stored->commit->object.oid.hash, index, index_nr, sha1_access);
+			oid_pos(&stored->commit->object.oid, index, index_nr, oid_access);
 
 		if (commit_pos < 0)
 			BUG("trying to write commit not in index");

rerere.c

@@ -11,6 +11,7 @@
 #include "pathspec.h"
 #include "object-store.h"
 #include "hash-lookup.h"
+#include "strmap.h"
 
 #define RESOLVED 0
 #define PUNTED 1
@@ -23,26 +24,27 @@ static int rerere_enabled = -1;
 /* automatically update cleanly resolved paths to the index */
 static int rerere_autoupdate;
 
-static int rerere_dir_nr;
-static int rerere_dir_alloc;
-
 #define RR_HAS_POSTIMAGE 1
 #define RR_HAS_PREIMAGE 2
-static struct rerere_dir {
+struct rerere_dir {
-	unsigned char hash[GIT_MAX_HEXSZ];
 	int status_alloc, status_nr;
 	unsigned char *status;
-} **rerere_dir;
+	char name[FLEX_ARRAY];
+};
+
+static struct strmap rerere_dirs = STRMAP_INIT;
 
 static void free_rerere_dirs(void)
 {
-	int i;
+	struct hashmap_iter iter;
-	for (i = 0; i < rerere_dir_nr; i++) {
+	struct strmap_entry *ent;
-		free(rerere_dir[i]->status);
+
-		free(rerere_dir[i]);
+	strmap_for_each_entry(&rerere_dirs, &iter, ent) {
+		struct rerere_dir *rr_dir = ent->value;
+		free(rr_dir->status);
+		free(rr_dir);
 	}
-	FREE_AND_NULL(rerere_dir);
+	strmap_clear(&rerere_dirs, 0);
-	rerere_dir_nr = rerere_dir_alloc = 0;
 }
 
 static void free_rerere_id(struct string_list_item *item)
@@ -52,7 +54,7 @@ static void free_rerere_id(struct string_list_item *item)
 
 static const char *rerere_id_hex(const struct rerere_id *id)
 {
-	return hash_to_hex(id->collection->hash);
+	return id->collection->name;
 }
 
 static void fit_variant(struct rerere_dir *rr_dir, int variant)
@@ -115,7 +117,7 @@ static int is_rr_file(const char *name, const char *filename, int *variant)
 static void scan_rerere_dir(struct rerere_dir *rr_dir)
 {
 	struct dirent *de;
-	DIR *dir = opendir(git_path("rr-cache/%s", hash_to_hex(rr_dir->hash)));
+	DIR *dir = opendir(git_path("rr-cache/%s", rr_dir->name));
 
 	if (!dir)
 		return;
@@ -133,39 +135,21 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
 	closedir(dir);
 }
 
-static const unsigned char *rerere_dir_hash(size_t i, void *table)
-{
-	struct rerere_dir **rr_dir = table;
-	return rr_dir[i]->hash;
-}
-
 static struct rerere_dir *find_rerere_dir(const char *hex)
 {
-	unsigned char hash[GIT_MAX_RAWSZ];
 	struct rerere_dir *rr_dir;
-	int pos;
 
-	if (get_sha1_hex(hex, hash))
+	rr_dir = strmap_get(&rerere_dirs, hex);
-		return NULL; /* BUG */
+	if (!rr_dir) {
-	pos = hash_pos(hash, rerere_dir, rerere_dir_nr, rerere_dir_hash);
+		FLEX_ALLOC_STR(rr_dir, name, hex);
-	if (pos < 0) {
-		rr_dir = xmalloc(sizeof(*rr_dir));
-		hashcpy(rr_dir->hash, hash);
 		rr_dir->status = NULL;
 		rr_dir->status_nr = 0;
 		rr_dir->status_alloc = 0;
-		pos = -1 - pos;
+		strmap_put(&rerere_dirs, hex, rr_dir);
 
-		/* Make sure the array is big enough ... */
-		ALLOC_GROW(rerere_dir, rerere_dir_nr + 1, rerere_dir_alloc);
-		/* ... and add it in. */
-		rerere_dir_nr++;
-		MOVE_ARRAY(rerere_dir + pos + 1, rerere_dir + pos,
-			   rerere_dir_nr - pos - 1);
-		rerere_dir[pos] = rr_dir;
 		scan_rerere_dir(rr_dir);
 	}
-	return rerere_dir[pos];
+	return rr_dir;
 }
 
 static int has_rerere_resolution(const struct rerere_id *id)
@@ -1178,6 +1162,14 @@ static void prune_one(struct rerere_id *id,
 		unlink_rr_item(id);
 }
 
+/* Does the basename in "path" look plausibly like an rr-cache entry? */
+static int is_rr_cache_dirname(const char *path)
+{
+	struct object_id oid;
+	const char *end;
+	return !parse_oid_hex(path, &oid, &end) && !*end;
+}
+
 void rerere_gc(struct repository *r, struct string_list *rr)
 {
 	struct string_list to_remove = STRING_LIST_INIT_DUP;
@@ -1205,10 +1197,11 @@ void rerere_gc(struct repository *r, struct string_list *rr)
 
 		if (is_dot_or_dotdot(e->d_name))
 			continue;
-		rr_dir = find_rerere_dir(e->d_name);
+		if (!is_rr_cache_dirname(e->d_name))
-		if (!rr_dir)
 			continue; /* or should we remove e->d_name? */
 
+		rr_dir = find_rerere_dir(e->d_name);
+
 		now_empty = 1;
 		for (id.variant = 0, id.collection = rr_dir;
 		     id.variant < id.collection->status_nr;

shallow.c

@@ -41,7 +41,7 @@ int register_shallow(struct repository *r, const struct object_id *oid)
 
 int unregister_shallow(const struct object_id *oid)
 {
-	int pos = commit_graft_pos(the_repository, oid->hash);
+	int pos = commit_graft_pos(the_repository, oid);
 	if (pos < 0)
 		return -1;
 	if (pos + 1 < the_repository->parsed_objects->grafts_nr)