From: Patrick Steinhardt Date: Mon, 12 Jan 2026 09:00:40 GMT Subject: [PATCH v5 0/7] Improvements for reading object info Message-ID: <20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im> In-Reply-To: <20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im> Hi, this patch series contains various small improvements for reading object info for either loose or packed objects. These improvements were split out of a larger patch series where I'm about to introduce a new generic `odb_for_each_object()` function. Changes in v5: - I discovered that this patch series incidentally fixes a segfault when using git-archive(1) to read deltified blobs that are larger than "core.bigFileThreshold". So the only change is an added test case that will detect this regression going forward. - Link to v4: https://lore.kernel.org/r/20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im Changes in v4: - Extend the fix for OI_LOOSE and refactor the whole function to have a single exit path as proposed by Karthik. This results in a lot more changes, but makes the function way easier to reason about going forward. - Link to v3: https://lore.kernel.org/r/20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im Changes in v3: - Fix a commit message typo. - Fix a function comment missing some words. - Link to v2: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im Changes in v2: - Rebase the series on top of master with jc/object-read-stream-fix merged into it. I've also evicted the patch that fixes the same underlying issue. - Improve the commit message that drops OI_DBCACHED to explain why this is a safe refactoring. - Link to v1: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im Thanks! Patrick --- Patrick Steinhardt (7): object-file: always set OI_LOOSE when reading object info packfile: always declare object info to be OI_PACKED packfile: extend `is_delta` field to allow for "unknown" state packfile: always populate pack-specific info when reading object info packfile: disentangle return value of `packed_object_info()` packfile: skip unpacking object header for disk size requests packfile: drop repository parameter from `packed_object_info()` builtin/cat-file.c | 3 +- builtin/pack-objects.c | 4 +- commit-graph.c | 2 +- object-file.c | 115 ++++++++++++++++++++++++++++++------------------- odb.h | 8 +++- pack-bitmap.c | 3 +- packfile.c | 61 +++++++++++++++----------- packfile.h | 7 ++- t/t5003-archive-zip.sh | 34 +++++++++++++++ 9 files changed, 158 insertions(+), 79 deletions(-) Range-diff versus v4: 1: 07f529a631 = 1: da9d514001 object-file: always set OI_LOOSE when reading object info 2: b547df2885 ! 2: c7b29f3789 packfile: always declare object info to be OI_PACKED @@ Commit message Drop the OI_DBCACHED enum completely. None of the callers seem to care about the distinction. + Note that this also fixes a segfault introduced in 8c1b84bc97 + (streaming: move logic to read packed objects streams into backend, + 2025-11-23), which refactors how we stream packed objects. The intent is + to only read packed objects in case they are stored non-deltified as + we'd otherwise have to deflate them first. But the check for whether or + not the object is stored as a delta was unconditionally done via + `oi.u.packed.is_delta`, which is only valid in case `oi.whence` is + `OI_PACKED`. But under some circumstances we got `OI_DBCACHED` here, + which means that none of the `oi.u.packed` fields were initialized at + all. Consequently, we assumed the object was not stored as a delta, and + then try to read the object from `oi.u.packed.pack`, which is a `NULL` + pointer and thus causes a segfault. + + Add a test case for this issue so that this cannot regress in the + future anymore. + + Reported-by: Matt Smiley Signed-off-by: Patrick Steinhardt ## odb.h ## @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p, out: unuse_pack(&w_curs); + + ## t/t5003-archive-zip.sh ## +@@ t/t5003-archive-zip.sh: check_zip with_untracked2 + check_added with_untracked2 untracked one/untracked + check_added with_untracked2 untracked two/untracked + ++test_expect_success 'git-archive --format=zip with bigFile delta chains' ' ++ test_when_finished rm -rf repo && ++ git init repo && ++ ( ++ cd repo && ++ test-tool genrandom foo 100000 >base && ++ { ++ cat base && ++ echo "trailing data" ++ } >delta-1 && ++ { ++ cat delta-1 && ++ echo "trailing data" ++ } >delta-2 && ++ git add . && ++ git commit -m "blobs" && ++ git repack -Ad && ++ git verify-pack -v .git/objects/pack/pack-*.idx >stats && ++ test_grep "chain length = 1: 1 object" stats && ++ test_grep "chain length = 2: 1 object" stats && ++ ++ git -c core.bigFileThreshold=1k archive --format=zip HEAD >archive.zip && ++ if test_have_prereq UNZIP ++ then ++ mkdir unpack && ++ cd unpack && ++ "$GIT_UNZIP" ../archive.zip && ++ test_cmp base ../base && ++ test_cmp delta-1 ../delta-1 && ++ test_cmp delta-2 ../delta-2 ++ fi ++ ) ++' ++ + # Test remote archive over HTTP protocol. + # + # Note: this should be the last part of this test suite, because 3: 28940ce932 = 3: ef5ac585f0 packfile: extend `is_delta` field to allow for "unknown" state 4: c13c74467d = 4: 2a844d61fe packfile: always populate pack-specific info when reading object info 5: d3c17fcc71 = 5: a23f59d530 packfile: disentangle return value of `packed_object_info()` 6: 1c598686c5 = 6: f246dc3745 packfile: skip unpacking object header for disk size requests 7: afc5d85991 = 7: a0c4f59547 packfile: drop repository parameter from `packed_object_info()` --- base-commit: 7df68b50e49b6a1b576abb19b2e5d457749bc28b change-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2