Re: [PATCH v3 1/7] object-file: always set OI_LOOSE when reading object info
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jan 7, 2026, 08:50 UTC
- Message-ID
- <CAOLa=ZSNmi_Lzb=3EdWks=mMOPvfijT2659y4YtxWnUKVUOXaA@mail.gmail.com>
- In-Reply-To
- <20260106-b4-pks-odb-read-object-info-improvements-v3-1-b5e02fae1fb0@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> There are some early returns in ``odb_source_loose_read_object_info()`
Nit: s/``/`
Show 8 quoted lines
> in cases where we don't have to open the loose object. These return > paths do not set `struct object_info::whence` to `OI_LOOSE` though, so > it becomes impossible for the caller to tell the format of such an > object. > > Nobody seems to care about this right now, but it's a bug waiting to > happen. Fix this by always setting `whence` on success. >
Show 37 quoted lines
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> object-file.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 6280e42f34..d566df427a 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -439,12 +439,23 @@ int odb_source_loose_read_object_info(struct odb_source *source,
> */
> if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
> struct stat st;
> - if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))
> - return quick_has_loose(source->loose, oid) ? 0 : -1;
> +
> + if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {
> + status = quick_has_loose(source->loose, oid) ? 0 : -1;
> + if (!status && oi)
> + oi->whence = OI_LOOSE;
> + return status;
> + }
> +
> if (stat_loose_object(source->loose, oid, &st, &path) < 0)
> return -1;
> - if (oi && oi->disk_sizep)
> - *oi->disk_sizep = st.st_size;
> +
> + if (oi) {
> + if (oi->disk_sizep)
> + *oi->disk_sizep = st.st_size;
> + oi->whence = OI_LOOSE;
> + }
> +
> return 0;
> }
>The change looks good. I'm wary of early returns independently doing the cleanup, wonder if it'd be better to do `status = ...; goto cleanup` instead.