[PATCH v2 2/5] odb/source: introduce error status when reading objects

The `read_object_info()` callback of `struct odb_source` is documented
to return a negative error code in case reading the object has failed,
and zero otherwise. This is overly broad though, as there are two very
different kinds of failures:

  - The object may not exist in the source at all.

  - The object exists, but reading it has failed, for example because
    its on-disk state is corrupt.

This distinction matters to callers: when an object is corrupt in one
source we may still find a good copy of it in another source, so we may
still be able to proceed with a given operation.

The "packed" source already distinguishes these cases by returning a
positive value for missing objects and a negative value in case reading
the object has failed. But it is the only such source that distinguishes
those cases, and the returned value is translated into a negative error
code by the "files" backend anyway.

Introduce a new error status that is specific to reading objects and
adapt the infrastructure to return it. For now, we only discern
successful reads from generic failures, which mostly matches the status
quo. In subsequent commits though we're about to add an error that
explicitly tells the caller that an object does not exist.

Note that we keep the "packed" backend as-is with its positive return
code for missing objects. This will be fixed in the next commit.

Signed-off-by: Patrick Steinhardt <[email protected]>
---
 odb.c                 | 16 ++++++++--------
 odb.h                 | 15 +++++++++++----
 odb/source-files.c    |  8 ++++----
 odb/source-inmemory.c |  8 ++++----
 odb/source-loose.c    |  8 ++++----
 odb/source-packed.c   |  8 ++++----
 odb/source.h          | 22 +++++++++++-----------
 7 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/odb.c b/odb.c
index caf1d0f542..1b37b26376 100644
--- a/odb.c
+++ b/odb.c
@@ -547,9 +547,9 @@ static int register_all_submodule_sources(struct object_database *odb)
 	return ret;
 }
 
-static int do_oid_object_info_extended(struct object_database *odb,
-				       const struct object_id *oid,
-				       struct object_info *oi, unsigned flags)
+static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,
+							const struct object_id *oid,
+							struct object_info *oi, unsigned flags)
 {
 	const struct object_id *real = oid;
 	int already_retried = 0;
@@ -696,12 +696,12 @@ static int oid_object_info_convert(struct repository *r,
 	return ret;
 }
 
-int odb_read_object_info_extended(struct object_database *odb,
-				  const struct object_id *oid,
-				  struct object_info *oi,
-				  enum object_info_flags flags)
+enum odb_read_status odb_read_object_info_extended(struct object_database *odb,
+						   const struct object_id *oid,
+						   struct object_info *oi,
+						   enum object_info_flags flags)
 {
-	int ret;
+	enum odb_read_status ret;
 
 	if (oid->algo && (hash_algo_by_ptr(odb->repo->hash_algo) != oid->algo))
 		return oid_object_info_convert(odb->repo, oid, oi, flags);
diff --git a/odb.h b/odb.h
index fca67e8253..43cbcc3aba 100644
--- a/odb.h
+++ b/odb.h
@@ -435,14 +435,21 @@ enum object_info_flags {
 	OBJECT_INFO_FOR_PREFETCH = (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK),
 };
 
+enum odb_read_status {
+	/* The read was successful. */
+	ODB_READ_OK = 0,
+	/* The read resulted in a generic error. */
+	ODB_READ_ERROR = -1,
+};
+
 /*
  * Read object info from the object database and populate the `object_info`
  * structure. Returns 0 on success, a negative error code otherwise.
  */
-int odb_read_object_info_extended(struct object_database *odb,
-				  const struct object_id *oid,
-				  struct object_info *oi,
-				  enum object_info_flags flags);
+enum odb_read_status odb_read_object_info_extended(struct object_database *odb,
+						   const struct object_id *oid,
+						   struct object_info *oi,
+						   enum object_info_flags flags);
 
 /*
  * Read a subset of object info for the given object ID. Returns an `enum
diff --git a/odb/source-files.c b/odb/source-files.c
index 5a68af7d84..a28aa5042d 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -59,10 +59,10 @@ static void odb_source_files_prepare(struct odb_source *source,
 	odb_source_prepare(&files->packed->base, flags);
 }
 
-static int odb_source_files_read_object_info(struct odb_source *source,
-					     const struct object_id *oid,
-					     struct object_info *oi,
-					     enum object_info_flags flags)
+static enum odb_read_status odb_source_files_read_object_info(struct odb_source *source,
+							      const struct object_id *oid,
+							      struct object_info *oi,
+							      enum object_info_flags flags)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
 
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index 3e71611b8e..53d2e3a852 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -56,10 +56,10 @@ static void populate_object_info(struct odb_source_inmemory *source,
 		oi->source_infop->source = &source->base;
 }
 
-static int odb_source_inmemory_read_object_info(struct odb_source *source,
-						const struct object_id *oid,
-						struct object_info *oi,
-						enum object_info_flags flags UNUSED)
+static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_source *source,
+								 const struct object_id *oid,
+								 struct object_info *oi,
+								 enum object_info_flags flags UNUSED)
 {
 	struct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);
 	const struct inmemory_object *object;
diff --git a/odb/source-loose.c b/odb/source-loose.c
index ef0e919277..ad8662842d 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -206,10 +206,10 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 	return ret;
 }
 
-static int odb_source_loose_read_object_info(struct odb_source *source,
-					     const struct object_id *oid,
-					     struct object_info *oi,
-					     enum object_info_flags flags)
+static enum odb_read_status odb_source_loose_read_object_info(struct odb_source *source,
+							      const struct object_id *oid,
+							      struct object_info *oi,
+							      enum object_info_flags flags)
 {
 	struct odb_source_loose *loose = odb_source_loose_downcast(source);
 	static struct strbuf buf = STRBUF_INIT;
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 16fa4f5769..dce68a57f7 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -35,10 +35,10 @@ static int find_pack_entry(struct odb_source_packed *store,
 	return 0;
 }
 
-static int odb_source_packed_read_object_info(struct odb_source *source,
-					      const struct object_id *oid,
-					      struct object_info *oi,
-					      enum object_info_flags flags)
+static enum odb_read_status odb_source_packed_read_object_info(struct odb_source *source,
+							       const struct object_id *oid,
+							       struct object_info *oi,
+							       enum object_info_flags flags)
 {
 	struct odb_source_packed *packed = odb_source_packed_downcast(source);
 	struct packed_git *bad_pack = NULL;
diff --git a/odb/source.h b/odb/source.h
index d69f8e2d1c..7b8ff3d19d 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -110,13 +110,13 @@ struct odb_source {
 	 *     second read in case they know that the first read would have
 	 *     already surfaced the object without reloading any on-disk state.
 	 *
-	 * The callback is expected to return a negative error code in case
-	 * reading the object has failed, 0 otherwise.
+	 * The callback is expected to return an `enum odb_read_status`. Please
+	 * refer to the individual values that can be returned.
 	 */
-	int (*read_object_info)(struct odb_source *source,
-				const struct object_id *oid,
-				struct object_info *oi,
-				enum object_info_flags flags);
+	enum odb_read_status (*read_object_info)(struct odb_source *source,
+						 const struct object_id *oid,
+						 struct object_info *oi,
+						 enum object_info_flags flags);
 
 	/*
 	 * This callback is expected to create a new read stream that can be
@@ -340,12 +340,12 @@ static inline void odb_source_prepare(struct odb_source *source,
 
 /*
  * Read an object from the object database source identified by its object ID.
- * Returns 0 on success, a negative error code otherwise.
+ * Please refer to `enum odb_read_status` for the individual error codes.
  */
-static inline int odb_source_read_object_info(struct odb_source *source,
-					      const struct object_id *oid,
-					      struct object_info *oi,
-					      enum object_info_flags flags)
+static inline enum odb_read_status odb_source_read_object_info(struct odb_source *source,
+							       const struct object_id *oid,
+							       struct object_info *oi,
+							       enum object_info_flags flags)
 {
 	return source->read_object_info(source, oid, oi, flags);
 }

-- 
2.55.0.822.g20453c30eb.dirty