From e279d645f1103e10a368660fef5e1fea7f59e27f Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 7 Mar 2026 22:48:15 +0000 Subject: [PATCH] oid: sha1s must now be zero-padded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we have two types of object IDs, with different sizes, we expect shorter object ID types (in other words, SHA1 object ids) to be zero-padded at their suffix. This allows us to use faster comparison and copy routines over the entirety of the structure, instead of trying to examine the type and then do a comparison of the appropriately sized structure. For pure manipulation of object IDs, this produces parity with the SHA1-only object ID code. SHA1: oid::cmp_sha1: 8.065 ms ± 703.9 μs / range: 7.875 ms … 14.88 ms (201 runs) oid::cmp_sha256: skipped oid::cpy_sha1: 5.340 ms ± 47.26 μs / range: 5.272 ms … 5.617 ms (548 runs) oid::cpy_sha256: skipped oid::zero_sha1: 5.327 ms ± 49.27 μs / range: 5.271 ms … 5.612 ms (553 runs) oid::zero_sha256: skipped SHA256 (before this change; testing the `type`): oid::cmp_sha1: 10.82 ms ± 1.029 ms / range: 10.57 ms … 20.63 ms (145 runs) oid::cmp_sha256: 10.63 ms ± 103.9 μs / range: 10.50 ms … 11.48 ms (279 runs) oid::cpy_sha1: 26.13 ms ± 63.91 μs / range: 26.07 ms … 26.45 ms (113 runs) oid::cpy_sha256: 20.92 ms ± 58.32 μs / range: 20.86 ms … 21.25 ms (141 runs) oid::zero_sha1: 13.19 ms ± 129.1 μs / range: 13.11 ms … 13.72 ms (224 runs) oid::zero_sha256: 13.12 ms ± 30.06 μs / range: 13.10 ms … 13.30 ms (225 runs) SHA256 (with this change): oid::cmp_sha1: 7.985 ms ± 562.3 μs / range: 7.874 ms … 14.32 ms (209 runs) oid::cmp_sha256: 6.609 ms ± 30.77 μs / range: 6.584 ms … 6.870 ms (443 runs) oid::cpy_sha1: 5.282 ms ± 21.90 μs / range: 5.266 ms … 5.524 ms (543 runs) oid::cpy_sha256: 5.279 ms ± 17.57 μs / range: 5.263 ms … 5.415 ms (554 runs) oid::zero_sha1: 5.288 ms ± 22.92 μs / range: 5.268 ms … 5.508 ms (544 runs) oid::zero_sha256: 5.286 ms ± 21.29 μs / range: 5.271 ms … 5.527 ms (542 runs) --- src/libgit2/indexer.c | 2 ++ src/libgit2/object.c | 3 +++ src/libgit2/odb.c | 1 + src/libgit2/odb_loose.c | 1 + src/libgit2/oid.c | 32 +++++++++----------------------- src/libgit2/oid.h | 15 ++++++--------- src/libgit2/pack-objects.c | 2 ++ src/libgit2/refdb_fs.c | 2 +- tests/libgit2/core/oid.c | 26 ++++++++++++++++++++++++++ 9 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/libgit2/indexer.c b/src/libgit2/indexer.c index 24e7a39bc..f1c7e437c 100644 --- a/src/libgit2/indexer.c +++ b/src/libgit2/indexer.c @@ -490,6 +490,8 @@ static int store_object(git_indexer *idx) pentry = git__calloc(1, sizeof(struct git_pack_entry)); GIT_ERROR_CHECK_ALLOC(pentry); + memset(&oid, 0, sizeof(git_oid)); + if (git_hash_final(oid.id, &idx->hash_ctx)) { git__free(pentry); goto on_error; diff --git a/src/libgit2/object.c b/src/libgit2/object.c index 75a22d0ac..ef382c5b1 100644 --- a/src/libgit2/object.c +++ b/src/libgit2/object.c @@ -700,6 +700,7 @@ static int id_from_fd( goto done; } + memset(out, 0, sizeof(git_oid)); error = git_hash_final(out->id, &ctx); #ifdef GIT_EXPERIMENTAL_SHA256 @@ -865,6 +866,8 @@ static int id_from_buffer( vec[1].data = (void *)data; vec[1].len = len; + memset(id, 0, sizeof(git_oid)); + #ifdef GIT_EXPERIMENTAL_SHA256 id->type = opts->oid_type; #endif diff --git a/src/libgit2/odb.c b/src/libgit2/odb.c index 4d740cf47..22ad10e79 100644 --- a/src/libgit2/odb.c +++ b/src/libgit2/odb.c @@ -1533,6 +1533,7 @@ int git_odb_stream_finalize_write(git_oid *out, git_odb_stream *stream) return git_odb_stream__invalid_length(stream, "stream_finalize_write()"); + memset(out, 0, sizeof(git_oid)); git_hash_final(out->id, stream->hash_ctx); #ifdef GIT_EXPERIMENTAL_SHA256 diff --git a/src/libgit2/odb_loose.c b/src/libgit2/odb_loose.c index dfdecd3c2..23246b462 100644 --- a/src/libgit2/odb_loose.c +++ b/src/libgit2/odb_loose.c @@ -739,6 +739,7 @@ GIT_INLINE(int) filename_to_oid(struct loose_backend *backend, git_oid *oid, con if (v < 0) return -1; + memset(oid, 0, sizeof(git_oid)); oid->id[0] = (unsigned char) v; ptr += 3; diff --git a/src/libgit2/oid.c b/src/libgit2/oid.c index db549f6ab..7f55e3e4d 100644 --- a/src/libgit2/oid.c +++ b/src/libgit2/oid.c @@ -47,10 +47,11 @@ int git_oid_from_prefix(git_oid *out, const char *str, size_t len, git_oid_t typ if (len > git_oid_hexsize(type)) return oid_error_invalid("too long"); + memset(out, 0, sizeof(git_oid)); + #ifdef GIT_EXPERIMENTAL_SHA256 out->type = type; #endif - memset(out->id, 0, size); for (p = 0; p < len; p++) { v = git__fromhex(str[p]); @@ -86,9 +87,12 @@ int git_oid_from_raw(git_oid *out, const unsigned char *raw, git_oid_t type) if (!(size = git_oid_size(type))) return oid_error_invalid("unknown type"); + memset(out, 0, sizeof(git_oid)); + #ifdef GIT_EXPERIMENTAL_SHA256 out->type = type; #endif + memcpy(out->id, raw, size); return 0; } @@ -231,16 +235,8 @@ int git_oid_fromraw(git_oid *out, const unsigned char *raw) int git_oid_cpy(git_oid *out, const git_oid *src) { - size_t size; - - if (!(size = git_oid_size(git_oid_type(src)))) - return oid_error_invalid("unknown type"); - -#ifdef GIT_EXPERIMENTAL_SHA256 - out->type = src->type; -#endif - - return git_oid_raw_cpy(out->id, src->id, size); + memcpy(out, src, sizeof(git_oid)); + return 0; } int git_oid_cmp(const git_oid *a, const git_oid *b) @@ -291,19 +287,9 @@ int git_oid_streq(const git_oid *oid_a, const char *str) return git_oid_strcmp(oid_a, str) == 0 ? 0 : -1; } -int git_oid_is_zero(const git_oid *oid_a) +int git_oid_is_zero(const git_oid *id) { - const unsigned char *a = oid_a->id; - size_t size = git_oid_size(git_oid_type(oid_a)); - -#ifdef GIT_EXPERIMENTAL_SHA256 - if (!oid_a->type) - return 1; - else if (!size) - return 0; -#endif - - return git_oid_raw_cmp(a, git_oid_zero, size) == 0 ? 1 : 0; + return memcmp(id->id, git_oid_zero, GIT_OID_MAX_SIZE) == 0 ? 1 : 0; } #ifndef GIT_DEPRECATE_HARD diff --git a/src/libgit2/oid.h b/src/libgit2/oid.h index cfcabce71..03c7bac53 100644 --- a/src/libgit2/oid.h +++ b/src/libgit2/oid.h @@ -221,19 +221,14 @@ GIT_INLINE(int) git_oid_raw_cpy( */ GIT_INLINE(int) git_oid__cmp(const git_oid *a, const git_oid *b) { -#ifdef GIT_EXPERIMENTAL_SHA256 - if (a->type != b->type) - return a->type - b->type; - - return git_oid_raw_cmp(a->id, b->id, git_oid_size(a->type)); -#else - return git_oid_raw_cmp(a->id, b->id, git_oid_size(GIT_OID_SHA1)); -#endif + return memcmp(a, b, sizeof(git_oid)); } GIT_INLINE(void) git_oid__cpy_prefix( git_oid *out, const git_oid *id, size_t len) { + memset(out, 0, sizeof(git_oid)); + #ifdef GIT_EXPERIMENTAL_SHA256 out->type = id->type; #endif @@ -258,10 +253,12 @@ GIT_INLINE(bool) git_oid__is_hexstr(const char *str, git_oid_t type) GIT_INLINE(void) git_oid_clear(git_oid *out, git_oid_t type) { - memset(out->id, 0, git_oid_size(type)); + memset(out, 0, sizeof(git_oid)); #ifdef GIT_EXPERIMENTAL_SHA256 out->type = type; +#else + GIT_UNUSED(type); #endif } diff --git a/src/libgit2/pack-objects.c b/src/libgit2/pack-objects.c index 0b975c7c1..52edd6e38 100644 --- a/src/libgit2/pack-objects.c +++ b/src/libgit2/pack-objects.c @@ -673,6 +673,8 @@ static int write_pack(git_packbuilder *pb, pb->nr_remaining -= pb->nr_written; } while (pb->nr_remaining && i < pb->nr_objects); + memset(&entry_oid, 0, sizeof(git_oid)); + if ((error = git_hash_final(entry_oid.id, &pb->ctx)) < 0) goto done; diff --git a/src/libgit2/refdb_fs.c b/src/libgit2/refdb_fs.c index 90092434b..1858a3407 100644 --- a/src/libgit2/refdb_fs.c +++ b/src/libgit2/refdb_fs.c @@ -1667,7 +1667,7 @@ static int maybe_append_head(refdb_fs_backend *backend, const git_reference *ref /* if we can't resolve, we use {0}*40 as old id */ if (git_reference_name_to_id(&old_id, backend->repo, ref->name) < 0) - memset(&old_id, 0, sizeof(old_id)); + git_oid_clear(&old_id, git_repository_oid_type(backend->repo)); if ((error = git_reference_lookup(&head, backend->repo, GIT_HEAD_REF)) < 0 || (error = reflog_append(backend, head, &old_id, git_reference_target(ref), who, message)) < 0) diff --git a/tests/libgit2/core/oid.c b/tests/libgit2/core/oid.c index 3b20061cb..4afc126e8 100644 --- a/tests/libgit2/core/oid.c +++ b/tests/libgit2/core/oid.c @@ -9,6 +9,8 @@ const char *str_oid_sha1 = "ae90f12eea699729ed24555e40b9fd669da12a12"; const char *str_oid_sha1_p = "ae90f12eea699729ed"; const char *str_oid_sha1_m = "ae90f12eea699729ed24555e40b9fd669da12a12THIS IS EXTRA TEXT THAT SHOULD GET IGNORED"; +const char *str_zero_sha1 = "0000000000000000000000000000000000000000"; + #ifdef GIT_EXPERIMENTAL_SHA256 static git_oid id_sha256; static git_oid idp_sha256; @@ -211,3 +213,27 @@ void test_core_oid__type_lookup(void) cl_assert_equal_s("unknown", git_oid_type_name(0)); cl_assert_equal_s("unknown", git_oid_type_name(42)); } + +void test_core_oid__zero_suffixed(void) +{ + git_oid id, zero; + + memset(&zero, 0x00, sizeof(git_oid)); + + memset(&id, 0xff, sizeof(git_oid)); + cl_git_pass(git_oid_from_string(&id, str_zero_sha1, GIT_OID_SHA1)); + +#ifdef GIT_EXPERIMENTAL_SHA256 + cl_assert(id.type == GIT_OID_SHA1); +#endif + cl_assert(memcmp(id.id, zero.id, GIT_OID_MAX_SIZE) == 0); + + + memset(&id, 0xff, sizeof(git_oid)); + cl_git_pass(git_oid_from_raw(&id, zero.id, GIT_OID_SHA1)); + +#ifdef GIT_EXPERIMENTAL_SHA256 + cl_assert(id.type == GIT_OID_SHA1); +#endif + cl_assert(memcmp(id.id, zero.id, GIT_OID_MAX_SIZE) == 0); +}