From 2d5942571c9056213e1e370be5e8d6b24fe8b148 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 3 Jan 2025 09:21:16 +0000 Subject: [PATCH 1/2] object: introduce `type_is_valid` There's no such thing as a "loose object type" or a "packed object type". There are only object types. Introduce `type_is_valid` and deprecate `typeisloose`. --- include/git2/deprecated.h | 10 ++++++++++ include/git2/object.h | 7 +++---- src/libgit2/object.c | 20 ++++++++++---------- src/libgit2/odb.c | 4 ++-- src/libgit2/odb_loose.c | 10 +++++----- tests/libgit2/object/raw/type2string.c | 26 +++++++++++++------------- 6 files changed, 43 insertions(+), 34 deletions(-) diff --git a/include/git2/deprecated.h b/include/git2/deprecated.h index d10143a5c..994503c7d 100644 --- a/include/git2/deprecated.h +++ b/include/git2/deprecated.h @@ -665,6 +665,16 @@ GIT_EXTERN(int) git_index_add_frombuffer( */ GIT_EXTERN(size_t) git_object__size(git_object_t type); +/** + * Determine if the given git_object_t is a valid object type. + * + * @deprecated use `git_object_type_is_valid` + * + * @param type object type to test. + * @return 1 if the type represents a valid object type, 0 otherwise + */ +GIT_EXTERN(int) git_object_typeisloose(git_object_t type); + /**@}*/ /** @name Deprecated Remote Functions diff --git a/include/git2/object.h b/include/git2/object.h index 8a50239fe..f923f81ed 100644 --- a/include/git2/object.h +++ b/include/git2/object.h @@ -180,13 +180,12 @@ GIT_EXTERN(const char *) git_object_type2string(git_object_t type); GIT_EXTERN(git_object_t) git_object_string2type(const char *str); /** - * Determine if the given git_object_t is a valid loose object type. + * Determine if the given git_object_t is a valid object type. * * @param type object type to test. - * @return true if the type represents a valid loose object type, - * false otherwise. + * @return 1 if the type represents a valid loose object type, 0 otherwise */ -GIT_EXTERN(int) git_object_typeisloose(git_object_t type); +GIT_EXTERN(int) git_object_type_is_valid(git_object_t type); /** * Recursively peel an object until an object of the specified type is met. diff --git a/src/libgit2/object.c b/src/libgit2/object.c index 1fff9de29..f20dbb6cf 100644 --- a/src/libgit2/object.c +++ b/src/libgit2/object.c @@ -33,7 +33,7 @@ typedef struct { } git_object_def; static git_object_def git_objects_table[] = { - /* 0 = GIT_OBJECT__EXT1 */ + /* 0 = unused */ { "", 0, NULL, NULL, NULL }, /* 1 = GIT_OBJECT_COMMIT */ @@ -46,14 +46,7 @@ static git_object_def git_objects_table[] = { { "blob", sizeof(git_blob), git_blob__parse, git_blob__parse_raw, git_blob__free }, /* 4 = GIT_OBJECT_TAG */ - { "tag", sizeof(git_tag), git_tag__parse, git_tag__parse_raw, git_tag__free }, - - /* 5 = GIT_OBJECT__EXT2 */ - { "", 0, NULL, NULL, NULL }, - /* 6 = GIT_OBJECT_OFS_DELTA */ - { "OFS_DELTA", 0, NULL, NULL, NULL }, - /* 7 = GIT_OBJECT_REF_DELTA */ - { "REF_DELTA", 0, NULL, NULL, NULL }, + { "tag", sizeof(git_tag), git_tag__parse, git_tag__parse_raw, git_tag__free } }; int git_object__from_raw( @@ -342,7 +335,7 @@ git_object_t git_object_stringn2type(const char *str, size_t len) return GIT_OBJECT_INVALID; } -int git_object_typeisloose(git_object_t type) +int git_object_type_is_valid(git_object_t type) { if (type < 0 || ((size_t) type) >= ARRAY_SIZE(git_objects_table)) return 0; @@ -350,6 +343,13 @@ int git_object_typeisloose(git_object_t type) return (git_objects_table[type].size > 0) ? 1 : 0; } +#ifndef GIT_DEPRECATE_HARD +int git_object_typeisloose(git_object_t type) +{ + return git_object_type_is_valid(type); +} +#endif + size_t git_object__size(git_object_t type) { if (type < 0 || ((size_t) type) >= ARRAY_SIZE(git_objects_table)) diff --git a/src/libgit2/odb.c b/src/libgit2/odb.c index 5678524c4..6ed072b35 100644 --- a/src/libgit2/odb.c +++ b/src/libgit2/odb.c @@ -116,7 +116,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj, git_oid_t oid_type) GIT_ASSERT_ARG(id); GIT_ASSERT_ARG(obj); - if (!git_object_typeisloose(obj->type)) { + if (!git_object_type_is_valid(obj->type)) { git_error_set(GIT_ERROR_INVALID, "invalid object type"); return -1; } @@ -219,7 +219,7 @@ int git_odb__hashfd( ssize_t read_len = 0; int error = 0; - if (!git_object_typeisloose(object_type)) { + if (!git_object_type_is_valid(object_type)) { git_error_set(GIT_ERROR_INVALID, "invalid object type for hash"); return -1; } diff --git a/src/libgit2/odb_loose.c b/src/libgit2/odb_loose.c index c772511a6..a91e296ae 100644 --- a/src/libgit2/odb_loose.c +++ b/src/libgit2/odb_loose.c @@ -243,7 +243,7 @@ static int read_loose_packlike(git_rawobj *out, git_str *obj) if ((error = parse_header_packlike(&hdr, &head_len, obj_data, obj_len)) < 0) goto done; - if (!git_object_typeisloose(hdr.type) || head_len > obj_len) { + if (!git_object_type_is_valid(hdr.type) || head_len > obj_len) { git_error_set(GIT_ERROR_ODB, "failed to inflate loose object"); error = -1; goto done; @@ -296,7 +296,7 @@ static int read_loose_standard(git_rawobj *out, git_str *obj) (error = parse_header(&hdr, &head_len, head, decompressed)) < 0) goto done; - if (!git_object_typeisloose(hdr.type)) { + if (!git_object_type_is_valid(hdr.type)) { git_error_set(GIT_ERROR_ODB, "failed to inflate disk object"); error = -1; goto done; @@ -436,7 +436,7 @@ static int read_header_loose(git_rawobj *out, git_str *loc) else error = read_header_loose_standard(out, obj, (size_t)obj_len); - if (!error && !git_object_typeisloose(out->type)) { + if (!error && !git_object_type_is_valid(out->type)) { git_error_set(GIT_ERROR_ZLIB, "failed to read loose object header"); error = -1; goto done; @@ -954,7 +954,7 @@ static int loose_backend__readstream_packlike( if ((error = parse_header_packlike(hdr, &head_len, data, data_len)) < 0) return error; - if (!git_object_typeisloose(hdr->type)) { + if (!git_object_type_is_valid(hdr->type)) { git_error_set(GIT_ERROR_ODB, "failed to inflate loose object"); return -1; } @@ -986,7 +986,7 @@ static int loose_backend__readstream_standard( (error = parse_header(hdr, &head_len, head, init)) < 0) return error; - if (!git_object_typeisloose(hdr->type)) { + if (!git_object_type_is_valid(hdr->type)) { git_error_set(GIT_ERROR_ODB, "failed to inflate disk object"); return -1; } diff --git a/tests/libgit2/object/raw/type2string.c b/tests/libgit2/object/raw/type2string.c index ebd81f552..6f0b47603 100644 --- a/tests/libgit2/object/raw/type2string.c +++ b/tests/libgit2/object/raw/type2string.c @@ -36,19 +36,19 @@ void test_object_raw_type2string__convert_string_to_type(void) cl_assert(git_object_string2type("hohoho") == GIT_OBJECT_INVALID); } -void test_object_raw_type2string__check_type_is_loose(void) +void test_object_raw_type2string__check_type_is_valid(void) { - cl_assert(git_object_typeisloose(GIT_OBJECT_INVALID) == 0); - cl_assert(git_object_typeisloose(0) == 0); /* EXT1 */ - cl_assert(git_object_typeisloose(GIT_OBJECT_COMMIT) == 1); - cl_assert(git_object_typeisloose(GIT_OBJECT_TREE) == 1); - cl_assert(git_object_typeisloose(GIT_OBJECT_BLOB) == 1); - cl_assert(git_object_typeisloose(GIT_OBJECT_TAG) == 1); - cl_assert(git_object_typeisloose(5) == 0); /* EXT2 */ - cl_assert(git_object_typeisloose(GIT_OBJECT_OFS_DELTA) == 0); - cl_assert(git_object_typeisloose(GIT_OBJECT_REF_DELTA) == 0); + cl_assert(git_object_type_is_valid(GIT_OBJECT_INVALID) == 0); + cl_assert(git_object_type_is_valid(0) == 0); /* EXT1 */ + cl_assert(git_object_type_is_valid(GIT_OBJECT_COMMIT) == 1); + cl_assert(git_object_type_is_valid(GIT_OBJECT_TREE) == 1); + cl_assert(git_object_type_is_valid(GIT_OBJECT_BLOB) == 1); + cl_assert(git_object_type_is_valid(GIT_OBJECT_TAG) == 1); + cl_assert(git_object_type_is_valid(5) == 0); /* EXT2 */ + cl_assert(git_object_type_is_valid(GIT_OBJECT_OFS_DELTA) == 0); + cl_assert(git_object_type_is_valid(GIT_OBJECT_REF_DELTA) == 0); - cl_assert(git_object_typeisloose(-2) == 0); - cl_assert(git_object_typeisloose(8) == 0); - cl_assert(git_object_typeisloose(1234) == 0); + cl_assert(git_object_type_is_valid(-2) == 0); + cl_assert(git_object_type_is_valid(8) == 0); + cl_assert(git_object_type_is_valid(1234) == 0); } From 23da3a8f3cc5594eae075f4fcf0d61f827236e7e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 3 Jan 2025 09:43:32 +0000 Subject: [PATCH 2/2] object: remove OFS_DELTA and REF_DELTA values Deltas are not objects, they're entries in a packfile. Remove them from the object enum. --- include/git2/types.h | 4 +--- src/libgit2/indexer.c | 8 ++++---- src/libgit2/pack-objects.c | 4 ++-- src/libgit2/pack.c | 20 ++++++++++---------- src/libgit2/pack.h | 4 ++++ tests/libgit2/object/raw/hash.c | 8 ++++---- tests/libgit2/object/raw/type2string.c | 20 ++++++++++---------- tests/libgit2/repo/hashfile.c | 2 +- 8 files changed, 36 insertions(+), 34 deletions(-) diff --git a/include/git2/types.h b/include/git2/types.h index a4afd18c3..fdccd9646 100644 --- a/include/git2/types.h +++ b/include/git2/types.h @@ -76,9 +76,7 @@ typedef enum { GIT_OBJECT_COMMIT = 1, /**< A commit object. */ GIT_OBJECT_TREE = 2, /**< A tree (directory listing) object. */ GIT_OBJECT_BLOB = 3, /**< A file revision object. */ - GIT_OBJECT_TAG = 4, /**< An annotated tag object. */ - GIT_OBJECT_OFS_DELTA = 6, /**< A delta, base is given by an offset. */ - GIT_OBJECT_REF_DELTA = 7 /**< A delta, base is given by object id. */ + GIT_OBJECT_TAG = 4 /**< An annotated tag object. */ } git_object_t; /** diff --git a/src/libgit2/indexer.c b/src/libgit2/indexer.c index f1c85cb88..cdd2b2431 100644 --- a/src/libgit2/indexer.c +++ b/src/libgit2/indexer.c @@ -319,9 +319,9 @@ static int advance_delta_offset(git_indexer *idx, git_object_t type) { git_mwindow *w = NULL; - GIT_ASSERT_ARG(type == GIT_OBJECT_REF_DELTA || type == GIT_OBJECT_OFS_DELTA); + GIT_ASSERT_ARG(type == GIT_PACKFILE_REF_DELTA || type == GIT_PACKFILE_OFS_DELTA); - if (type == GIT_OBJECT_REF_DELTA) { + if (type == GIT_PACKFILE_REF_DELTA) { idx->off += git_oid_size(idx->oid_type); } else { off64_t base_off; @@ -813,7 +813,7 @@ static int read_stream_object(git_indexer *idx, git_indexer_progress *stats) git_hash_init(&idx->hash_ctx); git_str_clear(&idx->entry_data); - if (type == GIT_OBJECT_REF_DELTA || type == GIT_OBJECT_OFS_DELTA) { + if (type == GIT_PACKFILE_REF_DELTA || type == GIT_PACKFILE_OFS_DELTA) { error = advance_delta_offset(idx, type); if (error == GIT_EBUFS) { idx->off = entry_start; @@ -1094,7 +1094,7 @@ static int fix_thin_pack(git_indexer *idx, git_indexer_progress *stats) if (error < 0) return error; - if (type == GIT_OBJECT_REF_DELTA) { + if (type == GIT_PACKFILE_REF_DELTA) { found_ref_delta = 1; break; } diff --git a/src/libgit2/pack-objects.c b/src/libgit2/pack-objects.c index ced98e8c8..efdd68aea 100644 --- a/src/libgit2/pack-objects.c +++ b/src/libgit2/pack-objects.c @@ -338,7 +338,7 @@ static int write_object( goto done; data_len = po->delta_size; - type = GIT_OBJECT_REF_DELTA; + type = GIT_PACKFILE_REF_DELTA; } else { if ((error = git_odb_read(&obj, pb->odb, &po->id)) < 0) goto done; @@ -354,7 +354,7 @@ static int write_object( (error = git_hash_update(&pb->ctx, hdr, hdr_len)) < 0) goto done; - if (type == GIT_OBJECT_REF_DELTA) { + if (type == GIT_PACKFILE_REF_DELTA) { if ((error = write_cb(po->delta->id.id, oid_size, cb_data)) < 0 || (error = git_hash_update(&pb->ctx, po->delta->id.id, oid_size)) < 0) goto done; diff --git a/src/libgit2/pack.c b/src/libgit2/pack.c index cf63b204e..a70975a75 100644 --- a/src/libgit2/pack.c +++ b/src/libgit2/pack.c @@ -390,7 +390,7 @@ int git_packfile__object_header(size_t *out, unsigned char *hdr, size_t size, gi unsigned char *hdr_base; unsigned char c; - GIT_ASSERT_ARG(type >= GIT_OBJECT_COMMIT && type <= GIT_OBJECT_REF_DELTA); + GIT_ASSERT_ARG(type >= GIT_OBJECT_COMMIT && type <= GIT_PACKFILE_REF_DELTA); /* TODO: add support for chunked objects; see git.git 6c0d19b1 */ @@ -532,7 +532,7 @@ int git_packfile_resolve_header( if (error < 0) return error; - if (type == GIT_OBJECT_OFS_DELTA || type == GIT_OBJECT_REF_DELTA) { + if (type == GIT_PACKFILE_OFS_DELTA || type == GIT_PACKFILE_REF_DELTA) { size_t base_size; git_packfile_stream stream; @@ -553,12 +553,12 @@ int git_packfile_resolve_header( base_offset = 0; } - while (type == GIT_OBJECT_OFS_DELTA || type == GIT_OBJECT_REF_DELTA) { + while (type == GIT_PACKFILE_OFS_DELTA || type == GIT_PACKFILE_REF_DELTA) { curpos = base_offset; error = git_packfile_unpack_header(&size, &type, p, &w_curs, &curpos); if (error < 0) return error; - if (type != GIT_OBJECT_OFS_DELTA && type != GIT_OBJECT_REF_DELTA) + if (type != GIT_PACKFILE_OFS_DELTA && type != GIT_PACKFILE_REF_DELTA) break; error = get_delta_base(&base_offset, p, &w_curs, &curpos, type, base_offset); @@ -635,7 +635,7 @@ static int pack_dependency_chain(git_dependency_chain *chain_out, elem->type = type; elem->base_key = obj_offset; - if (type != GIT_OBJECT_OFS_DELTA && type != GIT_OBJECT_REF_DELTA) + if (type != GIT_PACKFILE_OFS_DELTA && type != GIT_PACKFILE_REF_DELTA) break; error = get_delta_base(&base_offset, p, &w_curs, &curpos, type, obj_offset); @@ -675,7 +675,7 @@ int git_packfile_unpack( git_pack_cache_entry *cached = NULL; struct pack_chain_elem small_stack[SMALL_STACK_SIZE]; size_t stack_size = 0, elem_pos, alloclen; - git_object_t base_type; + int base_type; error = git_mutex_lock(&p->lock); if (error < 0) { @@ -735,8 +735,8 @@ int git_packfile_unpack( if (error < 0) goto cleanup; break; - case GIT_OBJECT_OFS_DELTA: - case GIT_OBJECT_REF_DELTA: + case GIT_PACKFILE_OFS_DELTA: + case GIT_PACKFILE_REF_DELTA: error = packfile_error("dependency chain ends in a delta"); goto cleanup; default: @@ -983,7 +983,7 @@ int get_delta_base( * than the hash size is stupid, as then a REF_DELTA would be * smaller to store. */ - if (type == GIT_OBJECT_OFS_DELTA) { + if (type == GIT_PACKFILE_OFS_DELTA) { unsigned used = 0; unsigned char c = base_info[used++]; size_t unsigned_base_offset = c & 127; @@ -1000,7 +1000,7 @@ int get_delta_base( return packfile_error("out of bounds"); base_offset = delta_obj_offset - unsigned_base_offset; *curpos += used; - } else if (type == GIT_OBJECT_REF_DELTA) { + } else if (type == GIT_PACKFILE_REF_DELTA) { git_oid base_oid; git_oid_from_raw(&base_oid, base_info, p->oid_type); diff --git a/src/libgit2/pack.h b/src/libgit2/pack.h index 4fd092149..e802d6074 100644 --- a/src/libgit2/pack.h +++ b/src/libgit2/pack.h @@ -33,6 +33,10 @@ typedef int git_pack_foreach_entry_offset_cb( #define PACK_SIGNATURE 0x5041434b /* "PACK" */ #define PACK_VERSION 2 #define pack_version_ok(v) ((v) == htonl(2)) + +#define GIT_PACKFILE_OFS_DELTA 6 +#define GIT_PACKFILE_REF_DELTA 7 + struct git_pack_header { uint32_t hdr_signature; uint32_t hdr_version; diff --git a/tests/libgit2/object/raw/hash.c b/tests/libgit2/object/raw/hash.c index 3bfaddaa2..914096f6a 100644 --- a/tests/libgit2/object/raw/hash.c +++ b/tests/libgit2/object/raw/hash.c @@ -87,16 +87,16 @@ void test_object_raw_hash__hash_junk_data(void) junk_obj.data = some_data; hash_object_fail(&id, &junk_obj); - junk_obj.type = 0; /* EXT1 */ + junk_obj.type = 0; /* unused */ hash_object_fail(&id, &junk_obj); - junk_obj.type = 5; /* EXT2 */ + junk_obj.type = 5; /* unused */ hash_object_fail(&id, &junk_obj); - junk_obj.type = GIT_OBJECT_OFS_DELTA; + junk_obj.type = 6; /* packfile offset delta */ hash_object_fail(&id, &junk_obj); - junk_obj.type = GIT_OBJECT_REF_DELTA; + junk_obj.type = 7; /* packfile ref delta */ hash_object_fail(&id, &junk_obj); junk_obj.type = 42; diff --git a/tests/libgit2/object/raw/type2string.c b/tests/libgit2/object/raw/type2string.c index 6f0b47603..e3a0764e5 100644 --- a/tests/libgit2/object/raw/type2string.c +++ b/tests/libgit2/object/raw/type2string.c @@ -7,14 +7,14 @@ void test_object_raw_type2string__convert_type_to_string(void) { cl_assert_equal_s(git_object_type2string(GIT_OBJECT_INVALID), ""); - cl_assert_equal_s(git_object_type2string(0), ""); /* EXT1 */ + cl_assert_equal_s(git_object_type2string(0), ""); /* unused */ cl_assert_equal_s(git_object_type2string(GIT_OBJECT_COMMIT), "commit"); cl_assert_equal_s(git_object_type2string(GIT_OBJECT_TREE), "tree"); cl_assert_equal_s(git_object_type2string(GIT_OBJECT_BLOB), "blob"); cl_assert_equal_s(git_object_type2string(GIT_OBJECT_TAG), "tag"); - cl_assert_equal_s(git_object_type2string(5), ""); /* EXT2 */ - cl_assert_equal_s(git_object_type2string(GIT_OBJECT_OFS_DELTA), "OFS_DELTA"); - cl_assert_equal_s(git_object_type2string(GIT_OBJECT_REF_DELTA), "REF_DELTA"); + cl_assert_equal_s(git_object_type2string(5), ""); /* unused */ + cl_assert_equal_s(git_object_type2string(6), ""); /* packfile offset delta */ + cl_assert_equal_s(git_object_type2string(7), ""); /* packfile ref delta */ cl_assert_equal_s(git_object_type2string(-2), ""); cl_assert_equal_s(git_object_type2string(8), ""); @@ -29,8 +29,8 @@ void test_object_raw_type2string__convert_string_to_type(void) cl_assert(git_object_string2type("tree") == GIT_OBJECT_TREE); cl_assert(git_object_string2type("blob") == GIT_OBJECT_BLOB); cl_assert(git_object_string2type("tag") == GIT_OBJECT_TAG); - cl_assert(git_object_string2type("OFS_DELTA") == GIT_OBJECT_OFS_DELTA); - cl_assert(git_object_string2type("REF_DELTA") == GIT_OBJECT_REF_DELTA); + cl_assert(git_object_string2type("OFS_DELTA") == GIT_OBJECT_INVALID); + cl_assert(git_object_string2type("REF_DELTA") == GIT_OBJECT_INVALID); cl_assert(git_object_string2type("CoMmIt") == GIT_OBJECT_INVALID); cl_assert(git_object_string2type("hohoho") == GIT_OBJECT_INVALID); @@ -39,14 +39,14 @@ void test_object_raw_type2string__convert_string_to_type(void) void test_object_raw_type2string__check_type_is_valid(void) { cl_assert(git_object_type_is_valid(GIT_OBJECT_INVALID) == 0); - cl_assert(git_object_type_is_valid(0) == 0); /* EXT1 */ + cl_assert(git_object_type_is_valid(0) == 0); /* unused */ cl_assert(git_object_type_is_valid(GIT_OBJECT_COMMIT) == 1); cl_assert(git_object_type_is_valid(GIT_OBJECT_TREE) == 1); cl_assert(git_object_type_is_valid(GIT_OBJECT_BLOB) == 1); cl_assert(git_object_type_is_valid(GIT_OBJECT_TAG) == 1); - cl_assert(git_object_type_is_valid(5) == 0); /* EXT2 */ - cl_assert(git_object_type_is_valid(GIT_OBJECT_OFS_DELTA) == 0); - cl_assert(git_object_type_is_valid(GIT_OBJECT_REF_DELTA) == 0); + cl_assert(git_object_type_is_valid(5) == 0); /* unused */ + cl_assert(git_object_type_is_valid(6) == 0); /* packfile offset delta */ + cl_assert(git_object_type_is_valid(7) == 0); /* packfile ref delta */ cl_assert(git_object_type_is_valid(-2) == 0); cl_assert(git_object_type_is_valid(8) == 0); diff --git a/tests/libgit2/repo/hashfile.c b/tests/libgit2/repo/hashfile.c index f053cb950..64b8dfa46 100644 --- a/tests/libgit2/repo/hashfile.c +++ b/tests/libgit2/repo/hashfile.c @@ -34,7 +34,7 @@ void test_repo_hashfile__simple(void) /* hash with invalid type */ cl_git_fail(git_odb__hashfile(&a, full.ptr, GIT_OBJECT_ANY, GIT_OID_SHA1)); - cl_git_fail(git_repository_hashfile(&b, _repo, full.ptr, GIT_OBJECT_OFS_DELTA, NULL)); + cl_git_fail(git_repository_hashfile(&b, _repo, full.ptr, 6, NULL)); git_str_dispose(&full); }