Re: [PATCH v1] path-walk: fix NULL pointer dereference in error message

[Derrick Stolee <stolee@gmail.com>]

On 3/20/2026 11:16 AM, D. Ben Knoble wrote:
> 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.
 
> 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