Commit fdfabdc4e26b42848b07e0e8e70fc51faceab976

Patrick Steinhardt 2018-12-01T09:49:10

strmap: remove legacy low-level interface Remove the low-level interface that was exposing implementation details of `git_strmap` to callers. From now on, only the high-level functions shall be used to retrieve or modify values of a map. Adjust remaining existing callers.

diff --git a/src/strmap.c b/src/strmap.c
index 1c7d5ef..c6e5b6d 100644
--- a/src/strmap.c
+++ b/src/strmap.c
@@ -43,9 +43,8 @@ size_t git_strmap_size(git_strmap *map)
 
 void *git_strmap_get(git_strmap *map, const char *key)
 {
-	size_t idx = git_strmap_lookup_index(map, key);
-	if (!git_strmap_valid_index(map, idx) ||
-	    !git_strmap_has_data(map, idx))
+	size_t idx = kh_get(str, map, key);
+	if (idx == kh_end(map) || !kh_exist(map, idx))
 		return NULL;
 	return kh_val(map, idx);
 }
@@ -69,10 +68,10 @@ int git_strmap_set(git_strmap *map, const char *key, void *value)
 
 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))
+	khiter_t idx = kh_get(str, map, key);
+	if (idx == kh_end(map))
 		return GIT_ENOTFOUND;
-	git_strmap_delete_at(map, idx);
+	kh_del(str, map, idx);
 	return 0;
 }
 
@@ -85,108 +84,17 @@ int git_strmap_iterate(void **value, git_strmap *map, size_t *iter, const char *
 {
 	size_t i = *iter;
 
-	while (i < git_strmap_end(map) && !git_strmap_has_data(map, i))
+	while (i < map->n_buckets && !kh_exist(map, i))
 		i++;
 
-	if (i >= git_strmap_end(map))
+	if (i >= map->n_buckets)
 		return GIT_ITEROVER;
 
 	if (key)
-		*key = git_strmap_key(map, i);
+		*key = kh_key(map, i);
 	if (value)
-		*value = git_strmap_value_at(map, i);
+		*value = kh_val(map, i);
 	*iter = ++i;
 
 	return 0;
 }
-
-size_t git_strmap_lookup_index(git_strmap *map, const char *key)
-{
-	return kh_get(str, map, key);
-}
-
-int git_strmap_valid_index(git_strmap *map, size_t idx)
-{
-	return idx != kh_end(map);
-}
-
-int git_strmap_has_data(git_strmap *map, size_t idx)
-{
-	return kh_exist(map, idx);
-}
-
-const char *git_strmap_key(git_strmap *map, size_t idx)
-{
-	return kh_key(map, idx);
-}
-
-void git_strmap_set_key_at(git_strmap *map, size_t idx, char *key)
-{
-	kh_val(map, idx) = key;
-}
-
-void *git_strmap_value_at(git_strmap *map, size_t idx)
-{
-	return kh_val(map, idx);
-}
-
-void git_strmap_set_value_at(git_strmap *map, size_t idx, void *value)
-{
-	kh_val(map, idx) = value;
-}
-
-void git_strmap_delete_at(git_strmap *map, size_t idx)
-{
-	kh_del(str, map, idx);
-}
-
-int git_strmap_put(git_strmap *map, const char *key, int *err)
-{
-	return kh_put(str, map, key, err);
-}
-
-void git_strmap_insert(git_strmap *map, const char *key, void *value, int *rval)
-{
-	khiter_t idx = kh_put(str, map, key, rval);
-
-	if ((*rval) >= 0) {
-		if ((*rval) == 0)
-			kh_key(map, idx) = key;
-		kh_val(map, idx) = value;
-	}
-}
-
-size_t git_strmap_begin(git_strmap *map)
-{
-	GIT_UNUSED(map);
-	return 0;
-}
-
-size_t git_strmap_end(git_strmap *map)
-{
-	return map->n_buckets;
-}
-
-int git_strmap_next(
-	void **data,
-	size_t* iter,
-	git_strmap *map)
-{
-	if (!map)
-		return GIT_ERROR;
-
-	while (*iter != git_strmap_end(map)) {
-		if (!(git_strmap_has_data(map, *iter))) {
-			++(*iter);
-			continue;
-		}
-
-		*data = git_strmap_value_at(map, *iter);
-
-		++(*iter);
-
-		return GIT_OK;
-	}
-
-	return GIT_ITEROVER;
-}
diff --git a/src/strmap.h b/src/strmap.h
index ce547ac..9f5e4cc 100644
--- a/src/strmap.h
+++ b/src/strmap.h
@@ -118,20 +118,6 @@ int git_strmap_exists(git_strmap *map, const char *key);
  */
 int git_strmap_iterate(void **value, git_strmap *map, size_t *iter, 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);
-
-int git_strmap_has_data(git_strmap *map, size_t idx);
-
-const char *git_strmap_key(git_strmap *map, size_t idx);
-void git_strmap_set_key_at(git_strmap *map, size_t idx, char *key);
-void *git_strmap_value_at(git_strmap *map, size_t idx);
-void git_strmap_set_value_at(git_strmap *map, size_t idx, void *value);
-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);
-
 #define git_strmap_foreach(h, kvar, vvar, code) { size_t __i = 0;		\
 	while (git_strmap_iterate((void **) &(vvar), h, &__i, &(kvar)) == 0) {	\
 		code;								\
@@ -142,12 +128,4 @@ void git_strmap_insert(git_strmap *map, const char *key, void *value, int *rval)
 		code;								\
 	} }
 
-size_t git_strmap_begin(git_strmap *map);
-size_t git_strmap_end(git_strmap *map);
-
-int git_strmap_next(
-	void **data,
-	size_t *iter,
-	git_strmap *map);
-
 #endif
diff --git a/tests/core/strmap.c b/tests/core/strmap.c
index d3a3939..ba118ae 100644
--- a/tests/core/strmap.c
+++ b/tests/core/strmap.c
@@ -22,7 +22,6 @@ void test_core_strmap__0(void)
 static void insert_strings(git_strmap *table, size_t count)
 {
 	size_t i, j, over;
-	int err;
 	char *str;
 
 	for (i = 0; i < count; ++i) {
@@ -35,17 +34,16 @@ static void insert_strings(git_strmap *table, size_t count)
 		for (j = 0, over = i / 26; over > 0; j++, over = over / 26)
 			str[j] = 'A' + (over % 26);
 
-		git_strmap_insert(table, str, str, &err);
-		cl_assert(err >= 0);
+		cl_git_pass(git_strmap_set(table, str, str));
 	}
 
 	cl_assert_equal_i(git_strmap_size(table), count);
 }
 
-void test_core_strmap__1(void)
+void test_core_strmap__inserted_strings_can_be_retrieved(void)
 {
-	int i;
 	char *str;
+	int i;
 
 	insert_strings(g_table, 20);
 
@@ -59,25 +57,18 @@ void test_core_strmap__1(void)
 	cl_assert(i == 20);
 }
 
-void test_core_strmap__2(void)
+void test_core_strmap__deleted_entry_cannot_be_retrieved(void)
 {
-	size_t pos;
-	int i;
 	char *str;
+	int i;
 
 	insert_strings(g_table, 20);
 
-	cl_assert(git_strmap_exists(g_table, "aaaaaaaaa"));
-	cl_assert(git_strmap_exists(g_table, "ggggggggg"));
-	cl_assert(!git_strmap_exists(g_table, "aaaaaaaab"));
-	cl_assert(!git_strmap_exists(g_table, "abcdefghi"));
-
 	cl_assert(git_strmap_exists(g_table, "bbbbbbbbb"));
-	pos = git_strmap_lookup_index(g_table, "bbbbbbbbb");
-	cl_assert(git_strmap_valid_index(g_table, pos));
-	cl_assert_equal_s(git_strmap_value_at(g_table, pos), "bbbbbbbbb");
-	free(git_strmap_value_at(g_table, pos));
-	git_strmap_delete_at(g_table, pos);
+	str = git_strmap_get(g_table, "bbbbbbbbb");
+	cl_assert_equal_s(str, "bbbbbbbbb");
+	cl_git_pass(git_strmap_delete(g_table, "bbbbbbbbb"));
+	free(str);
 
 	cl_assert(!git_strmap_exists(g_table, "bbbbbbbbb"));
 
@@ -86,10 +77,10 @@ void test_core_strmap__2(void)
 	cl_assert_equal_i(i, 19);
 }
 
-void test_core_strmap__3(void)
+void test_core_strmap__inserting_many_keys_succeeds(void)
 {
-	int i;
 	char *str;
+	int i;
 
 	insert_strings(g_table, 10000);
 
@@ -102,13 +93,10 @@ void test_core_strmap__get_succeeds_with_existing_entries(void)
 {
 	const char *keys[] = { "foo", "bar", "gobble" };
 	char *values[] = { "oof", "rab", "elbbog" };
-	int error;
 	size_t i;
 
-	for (i = 0; i < ARRAY_SIZE(keys); i++) {
-		git_strmap_insert(g_table, keys[i], values[i], &error);
-		cl_assert_equal_i(error, 1);
-	}
+	for (i = 0; i < ARRAY_SIZE(keys); i++)
+		cl_git_pass(git_strmap_set(g_table, keys[i], values[i]));
 
 	cl_assert_equal_s(git_strmap_get(g_table, "foo"), "oof");
 	cl_assert_equal_s(git_strmap_get(g_table, "bar"), "rab");
@@ -119,13 +107,10 @@ void test_core_strmap__get_returns_null_on_nonexisting_key(void)
 {
 	const char *keys[] = { "foo", "bar", "gobble" };
 	char *values[] = { "oof", "rab", "elbbog" };
-	int error;
 	size_t i;
 
-	for (i = 0; i < ARRAY_SIZE(keys); i++) {
-		git_strmap_insert(g_table, keys[i], values[i], &error);
-		cl_assert_equal_i(error, 1);
-	}
+	for (i = 0; i < ARRAY_SIZE(keys); i++)
+		cl_git_pass(git_strmap_set(g_table, keys[i], values[i]));
 
 	cl_assert_equal_p(git_strmap_get(g_table, "other"), NULL);
 }