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

From: Jeff King <peff@peff.net>
Date: Dec 12, 2022, 22:17 UTC
Message-ID: <Y5eoZV0nXVKbIPuP@coredump.intra.peff.net>
In-Reply-To: <20221212212947.1559820-1-jonathantanmy@google.com>

On Mon, Dec 12, 2022 at 01:29:47PM -0800, Jonathan Tan wrote:

Show 11 quoted lines

> > If we return a non-negative value, then we opened something, so by
> > definition, don't we have a path of the thing we opened?
> > 
> > I think the case Junio mentioned was if we for some reason didn't look
> > at _any_ path. In which case we'd be returning an error.
> 
> Ah, my reading comprehension is failing me, sorry. We do want "path"
> to point to something sensible (well, whenever we can) when an error
> occurs, though, since we want to include that path in our error message
> when DIE_IF_CORRUPT is used. So guaranteeing "path" when a non-negative
> value is returned (and hence, no error occurred) is not so useful.

But we only DIE_IF_CORRUPT when there is actual corruption, which means we've opened an object file, and "path" is valid.

The only time "path" would be invalid is if open_loose_object() itself returns an error, which is the message under discussion:

	fd = open_loose_object(r, oid, &path);
	if (fd < 0) {
		if (errno != ENOENT)
			error_errno(_("unable to open loose object %s"), path);
		return -1;
	}

If that stops expecting "path" to be valid (and just mentions the oid), then the rest of loose_object_info() should be fine.

-Peff