Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > + fd = open_loose_object(r, oid, &path); > + if (fd < 0) { > + if (errno != ENOENT) > + error_errno(_("unable to open loose object %s"), path); > + return -1; > + } I know there was a discussion in the previous round, but is this use of path truly safe? Currently it may happen to be as long as there is at least one element on the odb list, but when thinking things through with future-proofing point of view, I do not think assuming that path is always computable is a healthy thing to do in the longer term. Our "struct object_id" may be extended in the future and allow us to express "invalid" object name, in which case the error return we get may not even be about "loose object file not openable" but "there will never be a loose object file for such an invalid object name", in which case there won't be any path returned from the function. Other than that, the series looks quite clearly written. Nicely done. Thanks.