Re: [PATCH v4 3/4] object-file: emit corruption errors when detected
Junio C Hamano <gitster@pobox.com> writes:
Show 21 quoted lines
> Jonathan Tan <jonathantanmy@google.com> 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.Ah, good point. I think what I can do is to document the function to
only return a path if a path was involved in the error somehow, and make
anything that uses "path" in the caller check for NULL.
> Other than that, the series looks quite clearly written. Nicely
> done.
>
> Thanks.
Thanks for taking a look.