Commit 84a089da3701db370b2c77e6866abe3a5065c542

Patrick Steinhardt 2018-12-01T08:50:36

maps: provide return value when deleting entries Currently, the delete functions of maps do not provide a return value. Like this, it is impossible to tell whether the entry has really been deleted or not. Change the implementation to provide either a return value of zero if the entry has been successfully deleted or `GIT_ENOTFOUND` if the key could not be found. Convert callers to the `delete_at` functions to instead use this higher-level interface.

diff --git a/src/idxmap.c b/src/idxmap.c
index e3fcfb8..d7aa4ea 100644
--- a/src/idxmap.c
+++ b/src/idxmap.c
@@ -160,6 +160,24 @@ void git_idxmap_icase_insert(git_idxmap_icase *map, const git_index_entry *key, 
 	}
 }
 
+int git_idxmap_delete(git_idxmap *map, const git_index_entry *key)
+{
+	khiter_t idx = git_idxmap_lookup_index(map, key);
+	if (!git_idxmap_valid_index(map, idx))
+		return GIT_ENOTFOUND;
+	git_idxmap_delete_at(map, idx);
+	return 0;
+}
+
+int git_idxmap_icase_delete(git_idxmap_icase *map, const git_index_entry *key)
+{
+	khiter_t idx = git_idxmap_icase_lookup_index(map, key);
+	if (!git_idxmap_valid_index((git_idxmap *)map, idx))
+		return GIT_ENOTFOUND;
+	git_idxmap_icase_delete_at(map, idx);
+	return 0;
+}
+
 size_t git_idxmap_lookup_index(git_idxmap *map, const git_index_entry *key)
 {
 	return kh_get(idx, map, key);
@@ -204,16 +222,3 @@ void git_idxmap_icase_delete_at(git_idxmap_icase *map, size_t idx)
 {
 	kh_del(idxicase, map, idx);
 }
-
-void git_idxmap_delete(git_idxmap *map, const git_index_entry *key)
-{
-	khiter_t idx = git_idxmap_lookup_index(map, key);
-	if (git_idxmap_valid_index(map, idx))
-		git_idxmap_delete_at(map, idx);
-}
-void git_idxmap_icase_delete(git_idxmap_icase *map, const git_index_entry *key)
-{
-	khiter_t idx = git_idxmap_icase_lookup_index(map, key);
-	if (git_idxmap_valid_index((git_idxmap *)map, idx))
-		git_idxmap_icase_delete_at(map, idx);
-}
diff --git a/src/idxmap.h b/src/idxmap.h
index cf14254..ce9f084 100644
--- a/src/idxmap.h
+++ b/src/idxmap.h
@@ -146,6 +146,34 @@ int git_idxmap_set(git_idxmap *map, const git_index_entry *key, void *value);
  */
 int git_idxmap_icase_set(git_idxmap_icase *map, const git_index_entry *key, void *value);
 
+/**
+ * Delete an entry from the map.
+ *
+ * Delete the given key and its value from the map. If no such
+ * key exists, this will do nothing.
+ *
+ * @param map map to delete key in
+ * @param key key to delete
+ * @return `0` if the key has been deleted, GIT_ENOTFOUND if no
+ *         such key was found, a negative code in case of an
+ *         error
+ */
+int git_idxmap_delete(git_idxmap *map, const git_index_entry *key);
+
+/**
+ * Delete an entry from the map.
+ *
+ * Delete the given key and its value from the map. If no such
+ * key exists, this will do nothing.
+ *
+ * @param map map to delete key in
+ * @param key key to delete
+ * @return `0` if the key has been deleted, GIT_ENOTFOUND if no
+ *         such key was found, a negative code in case of an
+ *         error
+ */
+int git_idxmap_icase_delete(git_idxmap_icase *map, const git_index_entry *key);
+
 void git_idxmap_insert(git_idxmap *map, const git_index_entry *key, void *value, int *rval);
 void git_idxmap_icase_insert(git_idxmap_icase *map, const git_index_entry *key, void *value, int *rval);
 
@@ -160,7 +188,4 @@ int git_idxmap_icase_has_data(git_idxmap_icase *map, size_t idx);
 void git_idxmap_delete_at(git_idxmap *map, size_t idx);
 void git_idxmap_icase_delete_at(git_idxmap_icase *map, size_t idx);
 
-void git_idxmap_delete(git_idxmap *map, const git_index_entry *key);
-void git_idxmap_icase_delete(git_idxmap_icase *map, const git_index_entry *key);
-
 #endif
diff --git a/src/mwindow.c b/src/mwindow.c
index 09e219d..949e5fa 100644
--- a/src/mwindow.c
+++ b/src/mwindow.c
@@ -94,7 +94,6 @@ int git_mwindow_get_pack(struct git_pack_file **out, const char *path)
 void git_mwindow_put_pack(struct git_pack_file *pack)
 {
 	int count;
-	size_t pos;
 
 	if (git_mutex_lock(&git__mwindow_mutex) < 0)
 		return;
@@ -102,13 +101,12 @@ void git_mwindow_put_pack(struct git_pack_file *pack)
 	/* put before get would be a corrupted state */
 	assert(git__pack_cache);
 
-	pos = git_strmap_lookup_index(git__pack_cache, pack->pack_name);
 	/* if we cannot find it, the state is corrupted */
-	assert(git_strmap_valid_index(git__pack_cache, pos));
+	assert(git_strmap_exists(git__pack_cache, pack->pack_name));
 
 	count = git_atomic_dec(&pack->refcount);
 	if (count == 0) {
-		git_strmap_delete_at(git__pack_cache, pos);
+		git_strmap_delete(git__pack_cache, pack->pack_name);
 		git_packfile_free(pack);
 	}
 
diff --git a/src/offmap.c b/src/offmap.c
index 8a3bf49..eaf72ba 100644
--- a/src/offmap.c
+++ b/src/offmap.c
@@ -68,6 +68,14 @@ int git_offmap_set(git_offmap *map, const git_off_t key, void *value)
 	return 0;
 }
 
+int git_offmap_delete(git_offmap *map, const git_off_t key)
+{
+	khiter_t idx = git_offmap_lookup_index(map, key);
+	if (!git_offmap_valid_index(map, idx))
+		return GIT_ENOTFOUND;
+	git_offmap_delete_at(map, idx);
+	return 0;
+}
 
 size_t git_offmap_lookup_index(git_offmap *map, const git_off_t key)
 {
@@ -125,13 +133,6 @@ void git_offmap_insert(git_offmap *map, const git_off_t key, void *value, int *r
 	}
 }
 
-void git_offmap_delete(git_offmap *map, const git_off_t key)
-{
-	khiter_t idx = git_offmap_lookup_index(map, key);
-	if (git_offmap_valid_index(map, idx))
-		git_offmap_delete_at(map, idx);
-}
-
 size_t git_offmap_begin(git_offmap *map)
 {
 	GIT_UNUSED(map);
diff --git a/src/offmap.h b/src/offmap.h
index bf2ef7f..e7187d3 100644
--- a/src/offmap.h
+++ b/src/offmap.h
@@ -76,6 +76,20 @@ void *git_offmap_get(git_offmap *map, const git_off_t key);
  */
 int git_offmap_set(git_offmap *map, const git_off_t key, void *value);
 
+/**
+ * Delete an entry from the map.
+ *
+ * Delete the given key and its value from the map. If no such
+ * key exists, this will do nothing.
+ *
+ * @param map map to delete key in
+ * @param key key to delete
+ * @return `0` if the key has been deleted, GIT_ENOTFOUND if no
+ *         such key was found, a negative code in case of an
+ *         error
+ */
+int git_offmap_delete(git_offmap *map, const git_off_t key);
+
 size_t git_offmap_lookup_index(git_offmap *map, const git_off_t key);
 int git_offmap_valid_index(git_offmap *map, size_t idx);
 
@@ -89,7 +103,6 @@ void git_offmap_delete_at(git_offmap *map, size_t idx);
 
 int git_offmap_put(git_offmap *map, const git_off_t key, int *err);
 void git_offmap_insert(git_offmap *map, const git_off_t key, void *value, int *rval);
-void git_offmap_delete(git_offmap *map, const git_off_t key);
 
 size_t git_offmap_begin(git_offmap *map);
 size_t git_offmap_end(git_offmap *map);
diff --git a/src/oidmap.c b/src/oidmap.c
index 2890fdb..d96551f 100644
--- a/src/oidmap.c
+++ b/src/oidmap.c
@@ -74,6 +74,15 @@ int git_oidmap_set(git_oidmap *map, const git_oid *key, void *value)
 	return 0;
 }
 
+int git_oidmap_delete(git_oidmap *map, const git_oid *key)
+{
+	khiter_t idx = git_oidmap_lookup_index(map, key);
+	if (!git_oidmap_valid_index(map, idx))
+		return GIT_ENOTFOUND;
+	git_oidmap_delete_at(map, idx);
+	return 0;
+}
+
 size_t git_oidmap_lookup_index(git_oidmap *map, const git_oid *key)
 {
 	return kh_get(oid, map, key);
@@ -135,13 +144,6 @@ void git_oidmap_insert(git_oidmap *map, const git_oid *key, void *value, int *rv
 	}
 }
 
-void git_oidmap_delete(git_oidmap *map, const git_oid *key)
-{
-	khiter_t idx = git_oidmap_lookup_index(map, key);
-	if (git_oidmap_valid_index(map, idx))
-		git_oidmap_delete_at(map, idx);
-}
-
 size_t git_oidmap_begin(git_oidmap *map)
 {
 	GIT_UNUSED(map);
diff --git a/src/oidmap.h b/src/oidmap.h
index 39987d3..d8ffb92 100644
--- a/src/oidmap.h
+++ b/src/oidmap.h
@@ -76,6 +76,20 @@ void *git_oidmap_get(git_oidmap *map, const git_oid *key);
  */
 int git_oidmap_set(git_oidmap *map, const git_oid *key, void *value);
 
+/**
+ * Delete an entry from the map.
+ *
+ * Delete the given key and its value from the map. If no such
+ * key exists, this will do nothing.
+ *
+ * @param map map to delete key in
+ * @param key key to delete
+ * @return `0` if the key has been deleted, GIT_ENOTFOUND if no
+ *         such key was found, a negative code in case of an
+ *         error
+ */
+int git_oidmap_delete(git_oidmap *map, const git_oid *key);
+
 size_t git_oidmap_lookup_index(git_oidmap *map, const git_oid *key);
 int git_oidmap_valid_index(git_oidmap *map, size_t idx);
 
@@ -90,7 +104,6 @@ void git_oidmap_delete_at(git_oidmap *map, size_t idx);
 
 int git_oidmap_put(git_oidmap *map, const git_oid *key, int *err);
 void git_oidmap_insert(git_oidmap *map, const git_oid *key, void *value, int *rval);
-void git_oidmap_delete(git_oidmap *map, const git_oid *key);
 
 size_t git_oidmap_begin(git_oidmap *map);
 size_t git_oidmap_end(git_oidmap *map);
diff --git a/src/sortedcache.c b/src/sortedcache.c
index 326aacb..15ba6fd 100644
--- a/src/sortedcache.c
+++ b/src/sortedcache.c
@@ -358,9 +358,9 @@ int git_sortedcache_lookup_index(
 int git_sortedcache_remove(git_sortedcache *sc, size_t pos)
 {
 	char *item;
-	size_t mappos;
 
-	/* because of pool allocation, this can't actually remove the item,
+	/*
+	 * Because of pool allocation, this can't actually remove the item,
 	 * but we can remove it from the items vector and the hash table.
 	 */
 
@@ -371,8 +371,7 @@ int git_sortedcache_remove(git_sortedcache *sc, size_t pos)
 
 	(void)git_vector_remove(&sc->items, pos);
 
-	mappos = git_strmap_lookup_index(sc->map, item + sc->item_path_offset);
-	git_strmap_delete_at(sc->map, mappos);
+	git_strmap_delete(sc->map, item + sc->item_path_offset);
 
 	if (sc->free_item)
 		sc->free_item(sc->free_item_payload, item);
diff --git a/src/strmap.c b/src/strmap.c
index 4c1818a..ffdf6f1 100644
--- a/src/strmap.c
+++ b/src/strmap.c
@@ -67,6 +67,15 @@ int git_strmap_set(git_strmap *map, const char *key, void *value)
 	return 0;
 }
 
+int git_strmap_delete(git_strmap *map, const char *key)
+{
+	khiter_t idx = git_strmap_lookup_index(map, key);
+	if (!git_strmap_valid_index(map, idx))
+		return GIT_ENOTFOUND;
+	git_strmap_delete_at(map, idx);
+	return 0;
+}
+
 size_t git_strmap_lookup_index(git_strmap *map, const char *key)
 {
 	return kh_get(str, map, key);
@@ -128,13 +137,6 @@ void git_strmap_insert(git_strmap *map, const char *key, void *value, int *rval)
 	}
 }
 
-void git_strmap_delete(git_strmap *map, const char *key)
-{
-	khiter_t idx = git_strmap_lookup_index(map, key);
-	if (git_strmap_valid_index(map, idx))
-		git_strmap_delete_at(map, idx);
-}
-
 size_t git_strmap_begin(git_strmap *map)
 {
 	GIT_UNUSED(map);
diff --git a/src/strmap.h b/src/strmap.h
index efd875f..3317ea7 100644
--- a/src/strmap.h
+++ b/src/strmap.h
@@ -74,6 +74,20 @@ void *git_strmap_get(git_strmap *map, const char *key);
  */
 int git_strmap_set(git_strmap *map, const char *key, void *value);
 
+/**
+ * Delete an entry from the map.
+ *
+ * Delete the given key and its value from the map. If no such
+ * key exists, this will do nothing.
+ *
+ * @param map map to delete key in
+ * @param key key to delete
+ * @return `0` if the key has been deleted, GIT_ENOTFOUND if no
+ *         such key was found, a negative code in case of an
+ *         error
+ */
+int git_strmap_delete(git_strmap *map, const char *key);
+
 size_t git_strmap_lookup_index(git_strmap *map, const char *key);
 int git_strmap_valid_index(git_strmap *map, size_t idx);
 
@@ -88,7 +102,6 @@ void git_strmap_delete_at(git_strmap *map, size_t idx);
 
 int git_strmap_put(git_strmap *map, const char *key, int *err);
 void git_strmap_insert(git_strmap *map, const char *key, void *value, int *rval);
-void git_strmap_delete(git_strmap *map, const char *key);
 
 #define git_strmap_foreach(h, kvar, vvar, code) { size_t __i;			\
 	for (__i = git_strmap_begin(h); __i != git_strmap_end(h); ++__i) {	\