Re: [PATCH v1] path-walk: fix NULL pointer dereference in error message
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Mar 20, 2026, 16:14 UTC
- Message-ID
- <994f92e9-3576-455a-a142-0fefc559131c@gmail.com>
- In-Reply-To
- <CALnO6CDnwYaAPhp67kaYWtV48ULjWAR6ks1khVXmSs1oWUbRDQ@mail.gmail.com>
On 3/20/2026 11:16 AM, D. Ben Knoble wrote:
Show 19 quoted lines
> On Fri, Mar 20, 2026 at 7:50 AM Yuvraj Singh Chauhan
> <ysinghcin@gmail.com> wrote:
>> @@ -171,7 +171,7 @@ static int add_tree_entries(struct path_walk_context *ctx,
>>
>> if (!o) {
>> error(_("failed to find object %s"),
>> - oid_to_hex(&o->oid));
>> + oid_to_hex(&entry.oid));
>> return -1;
>> }
>>
>> --
>> 2.53.0.582.gca1db8a0f7
>
> Interesting find. I was hoping to see an easy way to reproduce hitting
> this code, and after grepping around a bit I found a few places that
> end up in this code (git-backfill and git-repo being the primary
> callers of walk_objects_by_path), but on second glance I think "!o" is
> current dead code.I can appreciate that the existing code is clearly incorrect, so tooling scanning code for defects would find this even if we can't easily create a test case to demonstrate it.
Show 5 quoted lines
> Still, fixing such obviously wrong dereference is good, but I wonder > if we should go further? > > You mentioned git-backfill with a tree missing from the local odb; do > you have a short reproduction script or test-case?
I imagine that it would be difficult to set up such a case, but maybe it would follow these steps (based on existing 'git backfill' tests that start with a partial clone):
1. Make a bare, blobless partial clone of the server repo.
2. Explode the client repo's object store into loose objects.
3. Delete a loose tree object, but one that isn't a commit's root tree. It must be a child tree.
In this case, we should hit this issue. Blobless partial clones expect all reachable trees to exist locally and so are not prepared to download missing trees on-demand.
Thanks, -Stolee