On Tue, Aug 19, 2025 at 10:05:25AM +0200, Patrick Steinhardt wrote: > On Mon, Aug 18, 2025 at 05:04:17PM -0400, Jeff King wrote: > > There's a call in describe_commit() to lookup_commit_reference(), but we > > don't check the return value. If it returns NULL, we'll segfault as we > > immediately dereference the result. > > > > In practice this can never happen, since all callers pass an oid which > > came from a "struct commit" already. So we can make this more obvious > > by just taking that commit struct in the first place. > > I was wondering a bit about commit-graphs. We had the case in the past > where it was possible to look up commits via the graph even though they > don't exist in the ODB. So we might actually end up with a missing > object if `GIT_COMMIT_GRAPH_PARANOIA=false`, which is the default value. > But that might be fine? No idea without digging further. I don't think existence matters here. Any call to parse_object() will call lookup_object() and find the same commit struct we already had, and then exit immediately because its "parsed" flag is set. So we'd never get a NULL return from lookup_commit_reference(). > In any case, the refactoring makes sense regardless from my point of > view. But yeah. Even if I am wrong above, this would fix it. ;) -Peff
