Jeff King <peff@xxxxxxxx> writes: > Actually, I think it is much worse than that. The code as written above > is already buggy (which is my fault, as I suggested it). > > In open_loose_object() we'll continue to iterate and pick out the "most > interesting errno". But we'll throw away the path that gave us that > errno. So we might well say: > > unable to open loose object /some/alternate/12/34abcd: permission denied > > when the actual problem is in /main/objdir/12/34abcd. > > It's fixable, but with some pain in handling the allocations. I think it > would be sufficient to just say: > > error_errno(_("unable to open loose object %s"), oid_to_hex(oid)); > > here. OK, let's go with this. > And possibly put a comment above open_loose_object() that "path" > is only guaranteed to point to something sensible when a non-negative > value is returned. Junio made a point that there could, for example, be no path when the odb list is empty (maybe in the future) so I don't think this would be sufficient. But there is already a comment there pointing to a comment in another function that states "path ... (if any)" so this is something that callers should already take care of. In my changes, I'll initialize it to NULL and whenever I use it, I'll check for non-NULL first.