From: Derrick Stolee Date: Fri, 20 Mar 2026 16:14:19 GMT Subject: Re: [PATCH v1] path-walk: fix NULL pointer dereference in error message Message-ID: <994f92e9-3576-455a-a142-0fefc559131c@gmail.com> In-Reply-To: On 3/20/2026 11:16 AM, D. Ben Knoble wrote: > On Fri, Mar 20, 2026 at 7:50 AM Yuvraj Singh Chauhan > 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