On Thu, Dec 08, 2022 at 12:57:07PM -0800, Jonathan Tan wrote:
> Note that in the RHS of this patch's diff, a check for ENOENT that was
> introduced in 3ba7a06552 (A loose object is not corrupt if it cannot
> be read due to EMFILE, 2010-10-28) is also removed. The purpose of this
> check is to avoid a false report of corruption if the errno contains
> something like EMFILE (or anything that is not ENOENT), in which case
> a more generic report is presented. Because, as of this patch, we no
> longer rely on such a heuristic to determine corruption, but surface
> the error message at the point when we read something that we did not
> expect, this check is no longer necessary.
I think this version still has the small issue that we'll _only_ surface
a generic error return in such a case, and never report EMFILE
specifically. I.e., I think we'd still want something like this on top:
diff --git a/object-file.c b/object-file.c
index dc7665d6fa..36082bc991 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1422,6 +1422,7 @@ static int loose_object_info(struct repository *r,
struct object_info *oi, int flags)
{
int status = 0;
+ int fd;
unsigned long mapsize;
const char *path = NULL;
void *map;
@@ -1455,7 +1456,13 @@ static int loose_object_info(struct repository *r,
return 0;
}
- map = map_loose_object_1(r, oid, &mapsize, &path);
+ fd = open_loose_object(r, oid, &path);
+ if (fd < 0) {
+ if (errno != ENOENT)
+ error_errno(_("unable to open loose object %s"), path);
+ return -1;
+ }
+ map = map_fd(fd, path, &mapsize);
if (!map)
return -1;
Otherwise ENOENT and EMFILE are indistinguishable from the user's
perspective. And one is normal and routine, but the other points to
something the user probably needs to fix.
-Peff
