Subject: Re: [PATCH v4 3/4] object-file: emit corruption errors when detected

From: Junio C Hamano <[email-masked]>
Date: Sat, 10 Dec 2022 09:16:42 +0900

Jonathan Tan <[email-masked]> 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.