diff options
author | Patrick Steinhardt <ps@pks.im> | 2019-01-23 10:48:55 +0100 |
---|---|---|
committer | Patrick Steinhardt <ps@pks.im> | 2019-02-15 13:16:48 +0100 |
commit | 2e0a304839764236654e73d38fa380b317a3fac1 (patch) | |
tree | fdba155593c31b1bb6260ad6bb09dba644bb8a56 | |
parent | 9694ef2064be3ecc3f173af296ab091f0498234d (diff) | |
download | libgit2-2e0a304839764236654e73d38fa380b317a3fac1.tar.gz |
oidmap: introduce high-level setter for key/value pairs
Currently, one would use either `git_oidmap_insert` to insert key/value pairs
into a map or `git_oidmap_put` to insert a key only. These function have
historically been macros, which is why their syntax is kind of weird: instead of
returning an error code directly, they instead have to be passed a pointer to
where the return value shall be stored. This does not match libgit2's common
idiom of directly returning error codes.Furthermore, `git_oidmap_put` is tightly
coupled with implementation details of the map as it exposes the index of
inserted entries.
Introduce a new function `git_oidmap_set`, which takes as parameters the map,
key and value and directly returns an error code. Convert all trivial callers of
`git_oidmap_insert` and `git_oidmap_put` to make use of it.
-rw-r--r-- | src/cache.c | 9 | ||||
-rw-r--r-- | src/describe.c | 9 | ||||
-rw-r--r-- | src/indexer.c | 42 | ||||
-rw-r--r-- | src/merge.c | 4 | ||||
-rw-r--r-- | src/odb_mempack.c | 12 | ||||
-rw-r--r-- | src/oidmap.c | 17 | ||||
-rw-r--r-- | src/oidmap.h | 15 | ||||
-rw-r--r-- | src/pack-objects.c | 28 | ||||
-rw-r--r-- | src/revwalk.c | 7 | ||||
-rw-r--r-- | tests/core/oidmap.c | 44 |
10 files changed, 120 insertions, 67 deletions
diff --git a/src/cache.c b/src/cache.c index ec3d9d17b..a7907836c 100644 --- a/src/cache.c +++ b/src/cache.c @@ -192,10 +192,7 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) /* not found */ if ((stored_entry = git_oidmap_get(cache->map, &entry->oid)) == NULL) { - int rval; - - git_oidmap_insert(cache->map, &entry->oid, entry, &rval); - if (rval >= 0) { + if (git_oidmap_set(cache->map, &entry->oid, entry) == 0) { git_cached_obj_incref(entry); cache->used_memory += entry->size; git_atomic_ssize_add(&git_cache__current_storage, (ssize_t)entry->size); @@ -209,12 +206,10 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry) entry = stored_entry; } else if (stored_entry->flags == GIT_CACHE_STORE_RAW && entry->flags == GIT_CACHE_STORE_PARSED) { - int rval; - git_cached_obj_decref(stored_entry); git_cached_obj_incref(entry); - git_oidmap_insert(cache->map, &entry->oid, entry, &rval); + git_oidmap_set(cache->map, &entry->oid, entry); } else { /* NO OP */ } diff --git a/src/describe.c b/src/describe.c index 62d8860b6..943fa55b5 100644 --- a/src/describe.c +++ b/src/describe.c @@ -119,13 +119,8 @@ static int add_to_known_names( e->path = git__strdup(path); git_oid_cpy(&e->peeled, peeled); - if (!found) { - int ret; - - git_oidmap_insert(names, &e->peeled, e, &ret); - if (ret < 0) - return -1; - } + if (!found && git_oidmap_set(names, &e->peeled, e) < 0) + return -1; } else git_tag_free(tag); diff --git a/src/indexer.c b/src/indexer.c index f898d206a..9ad78e82b 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -309,10 +309,8 @@ static int crc_object(uint32_t *crc_out, git_mwindow_file *mwf, git_off_t start, return 0; } -static void add_expected_oid(git_indexer *idx, const git_oid *oid) +static int add_expected_oid(git_indexer *idx, const git_oid *oid) { - int ret; - /* * If we know about that object because it is stored in our ODB or * because we have already processed it as part of our pack file, we do @@ -323,8 +321,10 @@ static void add_expected_oid(git_indexer *idx, const git_oid *oid) !git_oidmap_exists(idx->expected_oids, oid)) { git_oid *dup = git__malloc(sizeof(*oid)); git_oid_cpy(dup, oid); - git_oidmap_put(idx->expected_oids, dup, &ret); + return git_oidmap_set(idx->expected_oids, dup, NULL); } + + return 0; } static int check_object_connectivity(git_indexer *idx, const git_rawobj *obj) @@ -364,7 +364,8 @@ static int check_object_connectivity(git_indexer *idx, const git_rawobj *obj) size_t i; git_array_foreach(tree->entries, i, entry) - add_expected_oid(idx, entry->oid); + if (add_expected_oid(idx, entry->oid) < 0) + goto out; break; } @@ -375,9 +376,11 @@ static int check_object_connectivity(git_indexer *idx, const git_rawobj *obj) size_t i; git_array_foreach(commit->parent_ids, i, parent_oid) - add_expected_oid(idx, parent_oid); + if (add_expected_oid(idx, parent_oid) < 0) + goto out; - add_expected_oid(idx, &commit->tree_id); + if (add_expected_oid(idx, &commit->tree_id) < 0) + goto out; break; } @@ -385,7 +388,8 @@ static int check_object_connectivity(git_indexer *idx, const git_rawobj *obj) { git_tag *tag = (git_tag *) object; - add_expected_oid(idx, &tag->target); + if (add_expected_oid(idx, &tag->target) < 0) + goto out; break; } @@ -403,7 +407,6 @@ out: static int store_object(git_indexer *idx) { int i, error; - size_t k; git_oid oid; struct entry *entry; git_off_t entry_size; @@ -439,22 +442,18 @@ static int store_object(git_indexer *idx) git_oid_cpy(&pentry->sha1, &oid); pentry->offset = entry_start; - k = git_oidmap_put(idx->pack->idx_cache, &pentry->sha1, &error); - if (error == -1) { + if (git_oidmap_exists(idx->pack->idx_cache, &pentry->sha1)) { + git_error_set(GIT_ERROR_INDEXER, "duplicate object %s found in pack", git_oid_tostr_s(&pentry->sha1)); git__free(pentry); - git_error_set_oom(); goto on_error; } - if (error == 0) { - git_error_set(GIT_ERROR_INDEXER, "duplicate object %s found in pack", git_oid_tostr_s(&pentry->sha1)); + if ((error = git_oidmap_set(idx->pack->idx_cache, &pentry->sha1, pentry)) < 0) { git__free(pentry); + git_error_set_oom(); goto on_error; } - - git_oidmap_set_value_at(idx->pack->idx_cache, k, pentry); - git_oid_cpy(&entry->oid, &oid); if (crc_object(&entry->crc, &idx->pack->mwf, entry_start, entry_size) < 0) @@ -483,8 +482,7 @@ GIT_INLINE(bool) has_entry(git_indexer *idx, git_oid *id) static int save_entry(git_indexer *idx, struct entry *entry, struct git_pack_entry *pentry, git_off_t entry_start) { - int i, error; - size_t k; + int i; if (entry_start > UINT31_MAX) { entry->offset = UINT32_MAX; @@ -494,15 +492,13 @@ static int save_entry(git_indexer *idx, struct entry *entry, struct git_pack_ent } pentry->offset = entry_start; - k = git_oidmap_put(idx->pack->idx_cache, &pentry->sha1, &error); - if (error <= 0) { + if (git_oidmap_exists(idx->pack->idx_cache, &pentry->sha1) || + git_oidmap_set(idx->pack->idx_cache, &pentry->sha1, pentry) < 0) { git_error_set(GIT_ERROR_INDEXER, "cannot insert object into pack"); return -1; } - git_oidmap_set_value_at(idx->pack->idx_cache, k, pentry); - /* Add the object to the list */ if (git_vector_insert(&idx->objects, entry) < 0) return -1; diff --git a/src/merge.c b/src/merge.c index b8bb1947f..fbae70439 100644 --- a/src/merge.c +++ b/src/merge.c @@ -1108,7 +1108,6 @@ static int deletes_by_oid_enqueue(git_oidmap *map, git_pool* pool, const git_oid { deletes_by_oid_queue *queue; size_t *array_entry; - int error; if ((queue = git_oidmap_get(map, id)) == NULL) { queue = git_pool_malloc(pool, sizeof(deletes_by_oid_queue)); @@ -1118,8 +1117,7 @@ static int deletes_by_oid_enqueue(git_oidmap *map, git_pool* pool, const git_oid queue->next_pos = 0; queue->first_entry = idx; - git_oidmap_insert(map, id, queue, &error); - if (error < 0) + if (git_oidmap_set(map, id, queue) < 0) return -1; } else { array_entry = git_array_alloc(queue->arr); diff --git a/src/odb_mempack.c b/src/odb_mempack.c index e66f2e942..fc2302301 100644 --- a/src/odb_mempack.c +++ b/src/odb_mempack.c @@ -37,15 +37,9 @@ static int impl__write(git_odb_backend *_backend, const git_oid *oid, const void { struct memory_packer_db *db = (struct memory_packer_db *)_backend; struct memobject *obj = NULL; - size_t pos; size_t alloc_len; - int rval; - pos = git_oidmap_put(db->objects, oid, &rval); - if (rval < 0) - return -1; - - if (rval == 0) + if (git_oidmap_exists(db->objects, oid)) return 0; GIT_ERROR_CHECK_ALLOC_ADD(&alloc_len, sizeof(struct memobject), len); @@ -57,8 +51,8 @@ static int impl__write(git_odb_backend *_backend, const git_oid *oid, const void obj->len = len; obj->type = type; - git_oidmap_set_key_at(db->objects, pos, &obj->oid); - git_oidmap_set_value_at(db->objects, pos, obj); + if (git_oidmap_set(db->objects, &obj->oid, obj) < 0) + return -1; if (type == GIT_OBJECT_COMMIT) { struct memobject **store = git_array_alloc(db->commits); diff --git a/src/oidmap.c b/src/oidmap.c index 1e2b8124b..2890fdb28 100644 --- a/src/oidmap.c +++ b/src/oidmap.c @@ -57,6 +57,23 @@ void *git_oidmap_get(git_oidmap *map, const git_oid *key) return kh_val(map, idx); } +int git_oidmap_set(git_oidmap *map, const git_oid *key, void *value) +{ + size_t idx; + int rval; + + idx = kh_put(oid, map, key, &rval); + if (rval < 0) + return -1; + + if (rval == 0) + kh_key(map, idx) = key; + + kh_val(map, idx) = value; + + return 0; +} + size_t git_oidmap_lookup_index(git_oidmap *map, const git_oid *key) { return kh_get(oid, map, key); diff --git a/src/oidmap.h b/src/oidmap.h index 799fec402..39987d336 100644 --- a/src/oidmap.h +++ b/src/oidmap.h @@ -61,6 +61,21 @@ size_t git_oidmap_size(git_oidmap *map); */ void *git_oidmap_get(git_oidmap *map, const git_oid *key); +/** + * Set the entry for key to value. + * + * If the map has no corresponding entry for the given key, a new + * entry will be created with the given value. If an entry exists + * already, its value will be updated to match the given value. + * + * @param map map to create new entry in + * @param key key to set + * @param value value to associate the key with; may be NULL + * @return zero if the key was successfully set, a negative error + * code otherwise + */ +int git_oidmap_set(git_oidmap *map, const git_oid *key, void *value); + size_t git_oidmap_lookup_index(git_oidmap *map, const git_oid *key); int git_oidmap_valid_index(git_oidmap *map, size_t idx); diff --git a/src/pack-objects.c b/src/pack-objects.c index 094317a80..b2e2457d1 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -192,24 +192,26 @@ unsigned int git_packbuilder_set_threads(git_packbuilder *pb, unsigned int n) return pb->nr_threads; } -static void rehash(git_packbuilder *pb) +static int rehash(git_packbuilder *pb) { git_pobject *po; - size_t pos, i; - int ret; + size_t i; git_oidmap_clear(pb->object_ix); + for (i = 0, po = pb->object_list; i < pb->nr_objects; i++, po++) { - pos = git_oidmap_put(pb->object_ix, &po->id, &ret); - git_oidmap_set_value_at(pb->object_ix, pos, po); + if (git_oidmap_set(pb->object_ix, &po->id, po) < 0) + return -1; } + + return 0; } int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, const char *name) { git_pobject *po; - size_t newsize, pos; + size_t newsize; int ret; assert(pb && oid); @@ -233,7 +235,9 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, pb->object_list = git__reallocarray(pb->object_list, pb->nr_alloc, sizeof(*po)); GIT_ERROR_CHECK_ALLOC(pb->object_list); - rehash(pb); + + if (rehash(pb) < 0) + return -1; } po = pb->object_list + pb->nr_objects; @@ -246,13 +250,10 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, git_oid_cpy(&po->id, oid); po->hash = name_hash(name); - pos = git_oidmap_put(pb->object_ix, &po->id, &ret); - if (ret < 0) { + if (git_oidmap_set(pb->object_ix, &po->id, po) < 0) { git_error_set_oom(); - return ret; + return -1; } - assert(ret != 0); - git_oidmap_set_value_at(pb->object_ix, pos, po); pb->done = false; @@ -1541,7 +1542,8 @@ static int retrieve_object(struct walk_object **out, git_packbuilder *pb, const if ((error = lookup_walk_object(&obj, pb, id)) < 0) return error; - git_oidmap_insert(pb->walk_objects, &obj->id, obj, &error); + if ((error = git_oidmap_set(pb->walk_objects, &obj->id, obj)) < 0) + return error; } *out = obj; diff --git a/src/revwalk.c b/src/revwalk.c index a052a646d..6d12d34eb 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -21,8 +21,6 @@ git_commit_list_node *git_revwalk__commit_lookup( git_revwalk *walk, const git_oid *oid) { git_commit_list_node *commit; - size_t pos; - int ret; /* lookup and reserve space if not already present */ if ((commit = git_oidmap_get(walk->commits, oid)) != NULL) @@ -34,9 +32,8 @@ git_commit_list_node *git_revwalk__commit_lookup( git_oid_cpy(&commit->oid, oid); - pos = git_oidmap_put(walk->commits, &commit->oid, &ret); - assert(ret != 0); - git_oidmap_set_value_at(walk->commits, pos, commit); + if ((git_oidmap_set(walk->commits, &commit->oid, commit)) < 0) + return NULL; return commit; } diff --git a/tests/core/oidmap.c b/tests/core/oidmap.c index 0e66b7291..38b4c7d2a 100644 --- a/tests/core/oidmap.c +++ b/tests/core/oidmap.c @@ -182,3 +182,47 @@ void test_core_oidmap__get_fails_with_nonexisting_key(void) git_oidmap_free(map); } + +void test_core_oidmap__setting_oid_persists(void) +{ + git_oid oids[] = { + {{ 0x01 }}, + {{ 0x02 }}, + {{ 0x03 }} + }; + git_oidmap *map; + + cl_git_pass(git_oidmap_new(&map)); + cl_git_pass(git_oidmap_set(map, &oids[0], "one")); + cl_git_pass(git_oidmap_set(map, &oids[1], "two")); + cl_git_pass(git_oidmap_set(map, &oids[2], "three")); + + cl_assert_equal_s(git_oidmap_get(map, &oids[0]), "one"); + cl_assert_equal_s(git_oidmap_get(map, &oids[1]), "two"); + cl_assert_equal_s(git_oidmap_get(map, &oids[2]), "three"); + + git_oidmap_free(map); +} + +void test_core_oidmap__setting_existing_key_updates(void) +{ + git_oid oids[] = { + {{ 0x01 }}, + {{ 0x02 }}, + {{ 0x03 }} + }; + git_oidmap *map; + + cl_git_pass(git_oidmap_new(&map)); + cl_git_pass(git_oidmap_set(map, &oids[0], "one")); + cl_git_pass(git_oidmap_set(map, &oids[1], "two")); + cl_git_pass(git_oidmap_set(map, &oids[2], "three")); + cl_assert_equal_i(git_oidmap_size(map), 3); + + cl_git_pass(git_oidmap_set(map, &oids[1], "other")); + cl_assert_equal_i(git_oidmap_size(map), 3); + + cl_assert_equal_s(git_oidmap_get(map, &oids[1]), "other"); + + git_oidmap_free(map); +} |