From e6877b30d5dddce5720a15d724d7b381a5595915 Mon Sep 17 00:00:00 2001 From: Aaron Paterson Date: Mon, 16 Mar 2026 12:28:15 +0000 Subject: [PATCH 1/2] odb: add odb_source_files_try() for heterogeneous source iteration The odb_source vtable introduced in this release allows multiple backend implementations via the odb_source_type enum. However, source-chain iteration sites that need files-specific internals (pack store, loose cache, MIDX) currently use odb_source_files_downcast(), which calls BUG() for non-files source types. This makes it impossible to add a non-files source to the chain without crashing. Add odb_source_files_try() as a companion to the existing downcast function. It returns NULL for non-files sources instead of aborting. This follows the pattern used elsewhere in git where a "try" or "maybe" variant provides a fallback path while the strict version retains its safety guarantee. The existing odb_source_files_downcast() is unchanged and continues to BUG() on type mismatch, protecting call sites that should only ever receive a files source. A subsequent commit will convert the source-chain iteration sites to use this new helper. Signed-off-by: Aaron Paterson --- odb/source-files.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/odb/source-files.h b/odb/source-files.h index 23a3b4e04b1218..abd093b23f26ce 100644 --- a/odb/source-files.h +++ b/odb/source-files.h @@ -32,4 +32,18 @@ static inline struct odb_source_files *odb_source_files_downcast(struct odb_sour return container_of(source, struct odb_source_files, base); } +/* + * Try to cast the given source to the files backend. Returns NULL if + * the source uses a different backend. Use this in loops that iterate + * over heterogeneous source chains (e.g. when alternates may include + * non-files backends). Use odb_source_files_downcast() when the source + * is known to be a files backend. + */ +static inline struct odb_source_files *odb_source_files_try(struct odb_source *source) +{ + if (source->type != ODB_SOURCE_FILES) + return NULL; + return container_of(source, struct odb_source_files, base); +} + #endif From a8303b1427a41fd4b3ca107eabc49e8ac6d02410 Mon Sep 17 00:00:00 2001 From: Aaron Paterson Date: Mon, 16 Mar 2026 12:35:00 +0000 Subject: [PATCH 2/2] odb: use odb_source_files_try() in source-chain iterations Convert all source-chain iteration sites that access files-specific internals (pack store, loose cache, MIDX) to use odb_source_files_try() instead of odb_source_files_downcast(). These loops iterate the full source chain, which may include alternates. When a non-files backend is added to the chain (as an alternate or additional source), these sites must skip it rather than abort. The _try() helper returns NULL for non-files sources, and each site checks for NULL before accessing files-specific members. Call sites that access odb->sources directly (the primary source, which is always a files backend) or that are inside files-backend vtable callbacks continue to use odb_source_files_downcast() with its BUG() safety guarantee. Sites converted (22 across 11 files): - loose.c: loose object map loading and oid lookup - midx.c: multi-pack-index access and cleanup - object-file.c: loose cache and reprepare - object-name.c: short object filename search - packfile.c: window/fd management, pack entry lookup - packfile.h: repo_for_each_pack iterator macros - commit-graph.c: packed object enumeration - builtin/cat-file.c: batch object iteration - builtin/fast-import.c: pack lookup in store/stream - builtin/grep.c: pack preparation - builtin/pack-objects.c: cruft/kept pack enumeration Add a unit test verifying odb_source_files_try() returns the correct backend for ODB_SOURCE_FILES and NULL for other types. Note: repo_approximate_object_count() and has_object_pack() will not account for objects in non-files sources. These are display and optimization functions respectively, and a follow-up series can add vtable callbacks to address this. Signed-off-by: Aaron Paterson --- Makefile | 1 + builtin/cat-file.c | 9 ++++++--- builtin/fast-import.c | 8 ++++++-- builtin/grep.c | 4 +++- builtin/pack-objects.c | 15 +++++++++++---- commit-graph.c | 4 +++- loose.c | 12 +++++++++--- midx.c | 8 ++++++-- object-file.c | 20 ++++++++++++++------ object-name.c | 8 +++++--- packfile.c | 23 +++++++++++++++++------ packfile.h | 14 ++++++++++---- t/meson.build | 1 + t/unit-tests/u-odb-source.c | 25 +++++++++++++++++++++++++ 14 files changed, 117 insertions(+), 35 deletions(-) create mode 100644 t/unit-tests/u-odb-source.c diff --git a/Makefile b/Makefile index 58fb895f4e2f23..3d65093d5a3eb8 100644 --- a/Makefile +++ b/Makefile @@ -1544,6 +1544,7 @@ CLAR_TEST_SUITES += u-strvec CLAR_TEST_SUITES += u-trailer CLAR_TEST_SUITES += u-urlmatch-normalization CLAR_TEST_SUITES += u-utf8-width +CLAR_TEST_SUITES += u-odb-source CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X) CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES)) CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o diff --git a/builtin/cat-file.c b/builtin/cat-file.c index b6f12f41d6070a..0c63705b7233a8 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -882,9 +882,12 @@ static void batch_each_object(struct batch_options *opt, struct object_info oi = { 0 }; for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - int ret = packfile_store_for_each_object(files->packed, &oi, - batch_one_object_oi, &payload, flags); + struct odb_source_files *files = odb_source_files_try(source); + int ret; + if (!files) + continue; + ret = packfile_store_for_each_object(files->packed, &oi, + batch_one_object_oi, &payload, flags); if (ret) break; } diff --git a/builtin/fast-import.c b/builtin/fast-import.c index a41f95191e79aa..cef77ac9374e72 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -982,7 +982,9 @@ static int store_object( } for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; if (!packfile_list_find_oid(packfile_store_get_packs(files->packed), &oid)) continue; @@ -1189,7 +1191,9 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark) } for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; if (!packfile_list_find_oid(packfile_store_get_packs(files->packed), &oid)) continue; diff --git a/builtin/grep.c b/builtin/grep.c index e33285e5e69289..ea41236ce59071 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1219,7 +1219,9 @@ int cmd_grep(int argc, odb_prepare_alternates(the_repository->objects); for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; packfile_store_prepare(files->packed); } } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index cd013c0b68a6a3..229e3e40b03d8d 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1531,8 +1531,11 @@ static int want_cruft_object_mtime(struct repository *r, struct odb_source *source; for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct packed_git **cache = packfile_store_get_kept_pack_cache(files->packed, flags); + struct odb_source_files *files = odb_source_files_try(source); + struct packed_git **cache; + if (!files) + continue; + cache = packfile_store_get_kept_pack_cache(files->packed, flags); for (; *cache; cache++) { struct packed_git *p = *cache; @@ -1754,7 +1757,9 @@ static int want_object_in_pack_mtime(const struct object_id *oid, } for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; for (e = files->packed->packs.head; e; e = e->next) { struct packed_git *p = e->pack; @@ -4350,7 +4355,9 @@ static void add_objects_in_unpacked_packs(void) odb_prepare_alternates(to_pack.repo->objects); for (source = to_pack.repo->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; if (!source->local) continue; diff --git a/commit-graph.c b/commit-graph.c index f8e24145a513b0..b4254177357fbc 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1981,7 +1981,9 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx) odb_prepare_alternates(ctx->r->objects); for (source = ctx->r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; packfile_store_for_each_object(files->packed, &oi, add_packed_commits_oi, ctx, ODB_FOR_EACH_OBJECT_PACK_ORDER); } diff --git a/loose.c b/loose.c index 07333be6969fcc..1f036a8c2b90c0 100644 --- a/loose.c +++ b/loose.c @@ -64,10 +64,13 @@ static int insert_loose_map(struct odb_source *source, static int load_one_loose_object_map(struct repository *repo, struct odb_source *source) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT; FILE *fp; + if (!files) + return 0; + if (!files->loose->map) loose_object_map_init(&files->loose->map); if (!files->loose->cache) { @@ -235,8 +238,11 @@ int repo_loose_object_map_oid(struct repository *repo, khiter_t pos; for (source = repo->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct loose_object_map *loose_map = files->loose->map; + struct odb_source_files *files = odb_source_files_try(source); + struct loose_object_map *loose_map; + if (!files) + continue; + loose_map = files->loose->map; if (!loose_map) continue; map = (to == repo->compat_hash_algo) ? diff --git a/midx.c b/midx.c index ab8e2611d1fe58..d29b72f8d971fb 100644 --- a/midx.c +++ b/midx.c @@ -95,7 +95,9 @@ static int midx_read_object_offsets(const unsigned char *chunk_start, struct multi_pack_index *get_multi_pack_index(struct odb_source *source) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + return NULL; packfile_store_prepare(files->packed); return files->packed->midx; } @@ -806,7 +808,9 @@ void clear_midx_file(struct repository *r) struct odb_source *source; for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; if (files->packed->midx) close_midx(files->packed->midx); files->packed->midx = NULL; diff --git a/object-file.c b/object-file.c index a3ff7f586c91f3..11136ee1998ebc 100644 --- a/object-file.c +++ b/object-file.c @@ -1879,14 +1879,20 @@ static int append_loose_object(const struct object_id *oid, struct oidtree *odb_source_loose_cache(struct odb_source *source, const struct object_id *oid) { - struct odb_source_files *files = odb_source_files_downcast(source); - int subdir_nr = oid->hash[0]; + struct odb_source_files *files = odb_source_files_try(source); + int subdir_nr; struct strbuf buf = STRBUF_INIT; - size_t word_bits = bitsizeof(files->loose->subdir_seen[0]); - size_t word_index = subdir_nr / word_bits; - size_t mask = (size_t)1u << (subdir_nr % word_bits); + size_t word_bits, word_index, mask; uint32_t *bitmap; + if (!files) + return NULL; + + subdir_nr = oid->hash[0]; + word_bits = bitsizeof(files->loose->subdir_seen[0]); + word_index = subdir_nr / word_bits; + mask = (size_t)1u << (subdir_nr % word_bits); + if (subdir_nr < 0 || (size_t) subdir_nr >= bitsizeof(files->loose->subdir_seen)) BUG("subdir_nr out of range"); @@ -1919,7 +1925,9 @@ static void odb_source_loose_clear_cache(struct odb_source_loose *loose) void odb_source_loose_reprepare(struct odb_source *source) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + return; odb_source_loose_clear_cache(files->loose); } diff --git a/object-name.c b/object-name.c index 7b14c3bf9b9b48..09835a3225da4c 100644 --- a/object-name.c +++ b/object-name.c @@ -115,9 +115,11 @@ static void find_short_object_filename(struct disambiguate_state *ds) { struct odb_source *source; - for (source = ds->repo->objects->sources; source && !ds->ambiguous; source = source->next) - oidtree_each(odb_source_loose_cache(source, &ds->bin_pfx), - &ds->bin_pfx, ds->len, match_prefix, ds); + for (source = ds->repo->objects->sources; source && !ds->ambiguous; source = source->next) { + struct oidtree *tree = odb_source_loose_cache(source, &ds->bin_pfx); + if (tree) + oidtree_each(tree, &ds->bin_pfx, ds->len, match_prefix, ds); + } } static int match_hash(unsigned len, const unsigned char *a, const unsigned char *b) diff --git a/packfile.c b/packfile.c index 215a23e42be0fe..d7416e577f3f86 100644 --- a/packfile.c +++ b/packfile.c @@ -363,7 +363,9 @@ static int unuse_one_window(struct object_database *odb) struct pack_window *lru_w = NULL, *lru_l = NULL; for (source = odb->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; for (e = files->packed->packs.head; e; e = e->next) scan_windows(e->pack, &lru_p, &lru_w, &lru_l); } @@ -539,7 +541,9 @@ static int close_one_pack(struct repository *r) int accept_windows_inuse = 1; for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); + if (!files) + continue; for (e = files->packed->packs.head; e; e = e->next) { if (e->pack->pack_fd == -1) continue; @@ -1251,8 +1255,10 @@ const struct packed_git *has_packed_and_bad(struct repository *r, struct odb_source *source; for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); struct packfile_list_entry *e; + if (!files) + continue; for (e = files->packed->packs.head; e; e = e->next) if (oidset_contains(&e->pack->bad_objects, oid)) @@ -2268,8 +2274,11 @@ int has_object_pack(struct repository *r, const struct object_id *oid) odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - int ret = find_pack_entry(files->packed, oid, &e); + struct odb_source_files *files = odb_source_files_try(source); + int ret; + if (!files) + continue; + ret = find_pack_entry(files->packed, oid, &e); if (ret) return ret; } @@ -2284,8 +2293,10 @@ int has_object_kept_pack(struct repository *r, const struct object_id *oid, struct pack_entry e; for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); + struct odb_source_files *files = odb_source_files_try(source); struct packed_git **cache; + if (!files) + continue; cache = packfile_store_get_kept_pack_cache(files->packed, flags); diff --git a/packfile.h b/packfile.h index 8b04a258a7b9d5..3859415e4fb0de 100644 --- a/packfile.h +++ b/packfile.h @@ -193,8 +193,11 @@ static inline struct repo_for_each_pack_data repo_for_eack_pack_data_init(struct odb_prepare_alternates(repo->objects); for (struct odb_source *source = repo->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct packfile_list_entry *entry = packfile_store_get_packs(files->packed); + struct odb_source_files *files = odb_source_files_try(source); + struct packfile_list_entry *entry; + if (!files) + continue; + entry = packfile_store_get_packs(files->packed); if (!entry) continue; data.source = source; @@ -214,8 +217,11 @@ static inline void repo_for_each_pack_data_next(struct repo_for_each_pack_data * return; for (source = data->source->next; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct packfile_list_entry *entry = packfile_store_get_packs(files->packed); + struct odb_source_files *files = odb_source_files_try(source); + struct packfile_list_entry *entry; + if (!files) + continue; + entry = packfile_store_get_packs(files->packed); if (!entry) continue; data->source = source; diff --git a/t/meson.build b/t/meson.build index f66a73f8a07d93..e689ee49c7b63f 100644 --- a/t/meson.build +++ b/t/meson.build @@ -6,6 +6,7 @@ clar_test_suites = [ 'unit-tests/u-hashmap.c', 'unit-tests/u-list-objects-filter-options.c', 'unit-tests/u-mem-pool.c', + 'unit-tests/u-odb-source.c', 'unit-tests/u-oid-array.c', 'unit-tests/u-oidmap.c', 'unit-tests/u-oidtree.c', diff --git a/t/unit-tests/u-odb-source.c b/t/unit-tests/u-odb-source.c new file mode 100644 index 00000000000000..6f11c5342cc412 --- /dev/null +++ b/t/unit-tests/u-odb-source.c @@ -0,0 +1,25 @@ +#include "unit-test.h" +#include "odb/source.h" +#include "odb/source-files.h" + +/* + * Verify that odb_source_files_try() returns the files backend + * for ODB_SOURCE_FILES and NULL for other source types. + */ +void test_odb_source__try_returns_files_for_files_type(void) +{ + struct odb_source_files files_src; + memset(&files_src, 0, sizeof(files_src)); + files_src.base.type = ODB_SOURCE_FILES; + + cl_assert(odb_source_files_try(&files_src.base) == &files_src); +} + +void test_odb_source__try_returns_null_for_unknown_type(void) +{ + struct odb_source other_src; + memset(&other_src, 0, sizeof(other_src)); + other_src.type = ODB_SOURCE_UNKNOWN; + + cl_assert(odb_source_files_try(&other_src) == NULL); +}