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

Karthik Nayak <[email protected]> Thu, 20 Aug 2026 08:41:10 -0400
Newsgroups	org.kernel.vger.git
Message-ID	<CAOLa=ZSCf3CvTwtgj7RXncT6zPhyp4EX9r=g55uD+mTA1zp-5w@mail.gmail.com>
Patrick Steinhardt <[email protected]> writes:

> 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.
>

But isn't that the same for an object not existing in a source? If it
doesn't exist in one source, we may find a good copy of it in another?

> 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;
>  }
>

Here and elsewhere. Shouldn't we explicitly return ODB_READ_OK or
ODB_READ_ERROR instead of relying on implicit conversion?

[snip]
signature.asc (application/pgp-signature, 690 B)