From 894990788771bef6ca20398e46b217817b8e0db8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 10 Mar 2014 10:53:39 -0700 Subject: [PATCH 1/2] Fix a number of git_odb_exists_prefix bugs The git_odb_exists_prefix API was not dealing correctly when a later backend returned GIT_ENOTFOUND even if an earlier backend had found the object. Additionally, the unit tests were not properly exercising the API and had a couple mistakes in checking the results. Lastly, since the backends are not expected to behavior correctly unless all bytes of the short id are zero except for the prefix, this makes the ODB prefix APIs explicitly clear out the extra bytes so the user doesn't have to be as careful. --- src/odb.c | 24 +++++++++++++++++------- tests/odb/loose.c | 22 ++++++++++++---------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/odb.c b/src/odb.c index 085eda594..550951fd3 100644 --- a/src/odb.c +++ b/src/odb.c @@ -640,7 +640,7 @@ int git_odb_exists_prefix( { int error = GIT_ENOTFOUND, num_found = 0; size_t i; - git_oid last_found = {{0}}, found; + git_oid key = {{0}}, last_found = {{0}}, found; assert(db && short_id); @@ -659,6 +659,11 @@ int git_odb_exists_prefix( } } + /* just copy valid part of short_id */ + memcpy(&key.id, short_id->id, (len + 1) / 2); + if (len & 1) + key.id[len / 2] &= 0xF0; + for (i = 0; i < db->backends.length; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; @@ -666,7 +671,7 @@ int git_odb_exists_prefix( if (!b->exists_prefix) continue; - error = b->exists_prefix(&found, b, short_id, len); + error = b->exists_prefix(&found, b, &key, len); if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH) continue; if (error) @@ -683,11 +688,11 @@ int git_odb_exists_prefix( } if (!num_found) - return git_odb__error_notfound("no match for id prefix", short_id); + return git_odb__error_notfound("no match for id prefix", &key); if (out) git_oid_cpy(out, &last_found); - return error; + return 0; } int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git_oid *id) @@ -790,7 +795,7 @@ int git_odb_read_prefix( { size_t i; int error = GIT_ENOTFOUND; - git_oid found_full_oid = {{0}}; + git_oid key = {{0}}, found_full_oid = {{0}}; git_rawobj raw; void *data = NULL; bool found = false; @@ -809,13 +814,18 @@ int git_odb_read_prefix( return 0; } + /* just copy valid part of short_id */ + memcpy(&key.id, short_id->id, (len + 1) / 2); + if (len & 1) + key.id[len / 2] &= 0xF0; + for (i = 0; i < db->backends.length; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; if (b->read_prefix != NULL) { git_oid full_oid; - error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, short_id, len); + error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, &key, len); if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH) continue; @@ -836,7 +846,7 @@ int git_odb_read_prefix( } if (!found) - return git_odb__error_notfound("no match for prefix", short_id); + return git_odb__error_notfound("no match for prefix", &key); if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL) return -1; diff --git a/tests/odb/loose.c b/tests/odb/loose.c index ef7136e36..c91927c4a 100644 --- a/tests/odb/loose.c +++ b/tests/odb/loose.c @@ -66,23 +66,25 @@ void test_odb_loose__cleanup(void) void test_odb_loose__exists(void) { - git_oid id, id2; + git_oid id, id2; git_odb *odb; - write_object_files(&one); + write_object_files(&one); cl_git_pass(git_odb_open(&odb, "test-objects")); - cl_git_pass(git_oid_fromstr(&id, one.id)); + cl_git_pass(git_oid_fromstr(&id, one.id)); + cl_assert(git_odb_exists(odb, &id)); - cl_assert(git_odb_exists(odb, &id)); + cl_git_pass(git_oid_fromstrp(&id, "8b137891")); + cl_git_pass(git_odb_exists_prefix(&id2, odb, &id, 8)); + cl_assert_equal_i(0, git_oid_streq(&id2, one.id)); - cl_assert(git_odb_exists_prefix(&id2, odb, &id, 8)); - cl_assert(git_oid_equal(&id, &id2)); + /* Test for a missing object */ + cl_git_pass(git_oid_fromstr(&id, "8b137891791fe96927ad78e64b0aad7bded08baa")); + cl_assert(!git_odb_exists(odb, &id)); - /* Test for a non-existant object */ - cl_git_pass(git_oid_fromstr(&id2, "8b137891791fe96927ad78e64b0aad7bded08baa")); - cl_assert(!git_odb_exists(odb, &id2)); - cl_assert_equal_i(GIT_ENOTFOUND, git_odb_exists_prefix(NULL, odb, &id2, 8)); + cl_git_pass(git_oid_fromstrp(&id, "8b13789a")); + cl_assert_equal_i(GIT_ENOTFOUND, git_odb_exists_prefix(&id2, odb, &id, 8)); git_odb_free(odb); } From eb46fb2ba965f4e25946090dd172fcc3b20d93ee Mon Sep 17 00:00:00 2001 From: Jiri Pospisil Date: Sat, 8 Mar 2014 00:49:18 +0100 Subject: [PATCH 2/2] Add failing test for git_object_short_id --- tests/object/shortid.c | 7 +++++++ .../03/8d718da6a1ebbc6a7780a96ed75a70cc2ad6e2 | Bin 0 -> 23 bytes 2 files changed, 7 insertions(+) create mode 100644 tests/resources/duplicate.git/objects/03/8d718da6a1ebbc6a7780a96ed75a70cc2ad6e2 diff --git a/tests/object/shortid.c b/tests/object/shortid.c index fa1dac09a..d854cb78e 100644 --- a/tests/object/shortid.c +++ b/tests/object/shortid.c @@ -26,6 +26,13 @@ void test_object_shortid__select(void) cl_assert_equal_s("ce01362", shorty.ptr); git_object_free(obj); + git_oid_fromstr(&full, "038d718da6a1ebbc6a7780a96ed75a70cc2ad6e2"); + cl_git_pass(git_object_lookup(&obj, _repo, &full, GIT_OBJ_ANY)); + cl_git_pass(git_object_short_id(&shorty, obj)); + cl_assert_equal_i(7, shorty.size); + cl_assert_equal_s("038d718", shorty.ptr); + git_object_free(obj); + git_oid_fromstr(&full, "dea509d097ce692e167dfc6a48a7a280cc5e877e"); cl_git_pass(git_object_lookup(&obj, _repo, &full, GIT_OBJ_ANY)); cl_git_pass(git_object_short_id(&shorty, obj)); diff --git a/tests/resources/duplicate.git/objects/03/8d718da6a1ebbc6a7780a96ed75a70cc2ad6e2 b/tests/resources/duplicate.git/objects/03/8d718da6a1ebbc6a7780a96ed75a70cc2ad6e2 new file mode 100644 index 0000000000000000000000000000000000000000..7350d98a2325dbac9a8a9a1a474734ae0767abb8 GIT binary patch literal 23 fcmb$Jv literal 0 HcmV?d00001