From: Justin Tobler <jltobler@gmail.com>
Date: Wed, 19 Nov 2025 19:25:29 GMT
Subject: Re: [PATCH 03/18] streaming: propagate final object type via the stream
Message-ID: <cuvoz5gl7d6xgj757jgb26kj3qeunc4w3pg72it53zi6rs5lka@2nc5x4b2e3eg>
In-Reply-To: <20251119-b4-pks-odb-read-stream-v1-3-adacf03c2ccf@pks.im>

On 25/11/19 08:47AM, Patrick Steinhardt wrote:
> When opening the read stream for a specific object the caller is also
> expected to pass in a pointer to the object type. This type is passed
> down via multiple levels and will eventually be populated with the type
> of the looked-up object.
> 
> The way we propagate down the pointer though is somewhat non-obvious.
> While `istream_source()` still expects the pointer and looks it up via
> `odb_read_object_info_extended()`, we also pass it down even further
> into the format-specific callbacks that perform another lookup. This is
> quite confusing overall.
> 
> Refactor the code so that the responsibility to populate the object type
> rests solely with the format-specific callbacks. This will allow us to
> drop the call to `odb_read_object_info_extended()` in `istream_source()`
> entirely in a subsequent patch.
> 
> Furthermore, instead of propagating the type via an in-pointer, we now
> propagate the type via a new field in the object stream. It already has
> a `size` field, so it's only natural to have a second field that
> contains the object type.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  streaming.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/streaming.c b/streaming.c
> index 5ce6350123..9596a94c58 100644
> --- a/streaming.c
> +++ b/streaming.c
> @@ -33,6 +33,7 @@ struct odb_read_stream {
>  	close_istream_fn close;
>  	read_istream_fn read;
>  
> +	enum object_type type;

Now we are storing the object type in the stream. This avoids having to
pass the object type pointer around as much explictly. I think this is a
nice change.

>  	unsigned long size; /* inflated size of full object */
>  	git_zstream z;
>  	enum { z_unused, z_used, z_done, z_error } z_state;
> @@ -159,6 +160,7 @@ static struct odb_read_stream *attach_stream_filter(struct odb_read_stream *st,
>  	fs->o_end = fs->o_ptr = 0;
>  	fs->input_finished = 0;
>  	ifs->size = -1; /* unknown */
> +	ifs->type = st->type;
>  	return ifs;
>  }
>  
[snip]
> @@ -496,6 +495,7 @@ struct odb_read_stream *open_istream(struct repository *r,
>  	}
>  
>  	*size = st->size;
> +	*type = st->type;

So even though `open_istream()` returns `odb_read_stream` which contains
the object type, this function still accepts an object type pointer. At
first I thought this was a bit strange, but `odb_read_stream` is an
opaque structure so this make sense and is also what we do for object
size.

-Justin