odb/source: let callers discern missing and corrupt objects

As explained in the preceding commits, reading objects can either fail
because the object truly does not exist or because it exists, but its
data is corrupt. Some callers do care about this distinction, but there
is no way to tell these two cases apart right now.

Introduce a new `ODB_READ_NOT_FOUND` value that ought to be returned by
the backends in case the object truly does not exist and adapt backends
to use it.

Note that we don't yet return this error from `odb_read_object_info()`
itself. This will be fixed in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

odb.h

@@ -440,6 +440,8 @@ enum odb_read_status {
 	ODB_READ_OK = 0,
 	/* The read resulted in a generic error. */
 	ODB_READ_ERROR = -1,
+	/* The object could not be found. */
+	ODB_READ_NOT_FOUND = -2,
 };
 
 /*

odb/source-files.c

@@ -65,12 +65,26 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 							      enum object_info_flags flags)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
+	enum odb_read_status ret_packed, ret_loose;
 
-	if (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||
-	    !odb_source_read_object_info(&files->loose->base, oid, oi, flags))
+	ret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);
+	if (!ret_packed)
 		return 0;
 
-	return -1;
+	ret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);
+	if (!ret_loose)
+		return 0;
+
+	/*
+	 * Reading the packed object may have failed even though the object
+	 * exists, for example because it is corrupt. Report this failure to
+	 * the caller in case neither of the sources was able to read the
+	 * object, and prefer the error of the packed source in case both
+	 * reads have failed.
+	 */
+	if (ret_packed != ODB_READ_NOT_FOUND)
+		return ret_packed;
+	return ret_loose;
 }
 
 static int odb_source_files_read_object_stream(struct odb_read_stream **out,

odb/source-inmemory.c

@@ -66,7 +66,7 @@ static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_sour
 
 	object = find_cached_object(inmemory, oid);
 	if (!object)
-		return -1;
+		return ODB_READ_NOT_FOUND;
 
 	populate_object_info(inmemory, oi, object);
 	return 0;

odb/source-loose.c

@@ -91,11 +91,16 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 		struct stat st;
 
 		if ((!oi || (!oi->disk_sizep && !oi->mtimep)) && (flags & OBJECT_INFO_QUICK)) {
-			ret = quick_has_loose(loose, oid) ? 0 : -1;
+			ret = quick_has_loose(loose, oid) ? 0 : ODB_READ_NOT_FOUND;
 			goto out;
 		}
 
 		if (lstat(path, &st) < 0) {
+			if (errno == ENOENT) {
+				ret = ODB_READ_NOT_FOUND;
+				goto out;
+			}
+
 			ret = -1;
 			goto out;
 		}
@@ -113,9 +118,12 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 
 	fd = git_open(path);
 	if (fd < 0) {
-		if (errno != ENOENT)
-			error_errno(_("unable to open loose object %s"), oid_to_hex(oid));
-		ret = -1;
+		if (errno == ENOENT) {
+			ret = ODB_READ_NOT_FOUND;
+			goto out;
+		}
+
+		ret = error_errno(_("unable to open loose object %s"), oid_to_hex(oid));
 		goto out;
 	}
 
@@ -155,7 +163,7 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 
 		if (parse_loose_header(hdr, oi) < 0) {
 			ret = error(_("unable to parse %s header"), oid_to_hex(oid));
-			goto corrupt;
+			goto out;
 		}
 
 		if (*oi->typep < 0)
@@ -165,7 +173,7 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 			*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);
 			if (!*oi->contentp) {
 				ret = -1;
-				goto corrupt;
+				goto out;
 			}
 		}
 
@@ -173,21 +181,20 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 	case ULHR_BAD:
 		ret = error(_("unable to unpack %s header"),
 			    oid_to_hex(oid));
-		goto corrupt;
+		goto out;
 	case ULHR_TOO_LONG:
 		ret = error(_("header for %s too long, exceeds %d bytes"),
 			    oid_to_hex(oid), MAX_HEADER_LEN);
-		goto corrupt;
+		goto out;
 	}
 
 	ret = 0;
 
-corrupt:
-	if (ret && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
+out:
+	if (ret && ret != ODB_READ_NOT_FOUND && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
 		die(_("loose object %s (stored in %s) is corrupt"),
 		    oid_to_hex(oid), path);
 
-out:
 	if (stream_to_end)
 		git_inflate_end(stream_to_end);
 	if (map)
@@ -221,7 +228,7 @@ static enum odb_read_status odb_source_loose_read_object_info(struct odb_source
 	 * second time.
 	 */
 	if (flags & OBJECT_INFO_SECOND_READ)
-		return -1;
+		return ODB_READ_NOT_FOUND;
 
 	odb_loose_path(loose, &buf, oid);
 	return read_object_info_from_path(loose, buf.buf, oid, oi, flags);

odb/source-packed.c

@@ -61,7 +61,7 @@ static enum odb_read_status odb_source_packed_read_object_info(struct odb_source
 		 */
 		if (bad_pack)
 			return -1;
-		return 1;
+		return ODB_READ_NOT_FOUND;
 	}
 
 	/*

t/unit-tests/u-odb-inmemory.c

@@ -72,7 +72,8 @@ void test_odb_inmemory__read_missing_object(void)
 	const char *end;
 
 	cl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));
-	cl_must_fail(odb_source_read_object_info(&source->base, &oid, NULL, 0));
+	cl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0),
+			  ODB_READ_NOT_FOUND);
 
 	odb_source_free(&source->base);
 }