* [PATCH 2/2] odb: use odb_source_files_try() in source-chain iterations
2026-03-16 15:29 [PATCH 0/2] odb: add odb_source_files_try() for heterogeneous source iteration Aaron Paterson via GitGitGadget
2026-03-16 15:29 ` [PATCH 1/2] " Aaron Paterson via GitGitGadget
@ 2026-03-16 15:29 ` Aaron Paterson via GitGitGadget
2026-03-19 10:05 ` Patrick Steinhardt
1 sibling, 1 reply; 4+ messages in thread
From: Aaron Paterson via GitGitGadget @ 2026-03-16 15:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Aaron Paterson, Aaron Paterson
From: Aaron Paterson <apaterson@pm.me>
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 <apaterson@pm.me>
---
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 58fb895f4e..3d65093d5a 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 b6f12f41d6..0c63705b72 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 a41f95191e..cef77ac937 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 e33285e5e6..ea41236ce5 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 cd013c0b68..229e3e40b0 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 f8e24145a5..b425417735 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 07333be696..1f036a8c2b 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 ab8e2611d1..d29b72f8d9 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 a3ff7f586c..11136ee199 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 7b14c3bf9b..09835a3225 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 215a23e42b..d7416e577f 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 8b04a258a7..3859415e4f 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 f66a73f8a0..e689ee49c7 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 0000000000..6f11c5342c
--- /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);
+}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 4+ messages in thread