Re: [PATCH 05/18] streaming: allocate stream inside the backend-specific logic

> When creating a new stream we first allocate it and then call into
> backend-specific logic to populate the stream. This design requires that
> the stream itself contains a `union` with backend-specific members that
> then ultimately get populated by the backend-specific logic.
>
> This works, but it's awkward in the context of pluggable object
> databases. Each backend will need its own member in that union, and as
> the structure itself is completely opaque (it's only defined in
> "streamgin.c") it also has the consequence that we must have the logic

> that is specific to backends in "streaming.c".
>
> Ideally though, the infrastructure would be reversed: we have a generic
> `struct odb_read_stream` and some helper functions in "streaming.c",
> whereas the backend-specific logic sits in the backend's subsystem
> itself.
>

> This can be realized by using a design that is similar to how we handle
> reference databases: instead of having a union of members, we instead
> have backend-specific structures with a `struct odb_read_stream base`
> as its first member. The backends would thus hand out the pointer to the
> base, but internally they know to cast back to the backend-specific
> type.
>

> This means though that we need to allocate different structures
> depending on the backend. To prepare for this, move allocation of the
> structure into the backend-specific functions that open a new stream.
> Subsequent commits will then create those new backend-specific structs.
>

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  streaming.c | 99 +++++++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 63 insertions(+), 36 deletions(-)
>
> diff --git a/streaming.c b/streaming.c
> index d7db446d25..b8ce82483f 100644
> --- a/streaming.c
> +++ b/streaming.c
> @@ -222,27 +222,34 @@ static int close_istream_loose(struct odb_read_stream *st)
>  	return 0;
>  }
>
> -static int open_istream_loose(struct odb_read_stream *st, struct repository *r,
> +static int open_istream_loose(struct odb_read_stream **out,
> +			      struct repository *r,

>  			      const struct object_id *oid)
>  {
>  	struct object_info oi = OBJECT_INFO_INIT;
> +	struct odb_read_stream *st;
>  	struct odb_source *source;
> -
> -	oi.sizep = &st->size;
> -	oi.typep = &st->type;
> +	unsigned long mapsize;
> +	void *mapped;
>
>  	odb_prepare_alternates(r->objects);
>  	for (source = r->objects->sources; source; source = source->next) {
> -		st->u.loose.mapped = odb_source_loose_map_object(source, oid,
> -								 &st->u.loose.mapsize);
> -		if (st->u.loose.mapped)
> +		mapped = odb_source_loose_map_object(source, oid, &mapsize);
> +		if (mapped)
>  			break;
>  	}
> -	if (!st->u.loose.mapped)
> +	if (!mapped)
>  		return -1;
>
> -	switch (unpack_loose_header(&st->z, st->u.loose.mapped,
> -				    st->u.loose.mapsize, st->u.loose.hdr,
> +	/*
> +	 * Note: we must allocate this structure early even though we may still
> +	 * fail. This is because we need to initialize the zlib stream, and it
> +	 * is not possible to copy the stream around after the fact because it
> +	 * has self-referencing pointers.
> +	 */
> +	CALLOC_ARRAY(st, 1);
> +
> +	switch (unpack_loose_header(&st->z, mapped, mapsize, st->u.loose.hdr,
>  				    sizeof(st->u.loose.hdr))) {
>  	case ULHR_OK:
>  		break;
> @@ -250,19 +257,28 @@ static int open_istream_loose(struct odb_read_stream *st, struct repository *r,
>  	case ULHR_TOO_LONG:
>  		goto error;
>  	}
> +
> +	oi.sizep = &st->size;
> +	oi.typep = &st->type;
> +
>  	if (parse_loose_header(st->u.loose.hdr, &oi) < 0 || st->type < 0)
>  		goto error;
>
> +	st->u.loose.mapped = mapped;
> +	st->u.loose.mapsize = mapsize;
>  	st->u.loose.hdr_used = strlen(st->u.loose.hdr) + 1;
>  	st->u.loose.hdr_avail = st->z.total_out;
>  	st->z_state = z_used;
>  	st->close = close_istream_loose;
>  	st->read = read_istream_loose;
>
> +	*out = st;
> +
>  	return 0;
>  error:
>  	git_inflate_end(&st->z);
>  	munmap(st->u.loose.mapped, st->u.loose.mapsize);
> +	free(st);
>  	return -1;
>  }
>
> @@ -338,12 +354,16 @@ static int close_istream_pack_non_delta(struct odb_read_stream *st)
>  	return 0;
>  }
>
> -static int open_istream_pack_non_delta(struct odb_read_stream *st,
> +static int open_istream_pack_non_delta(struct odb_read_stream **out,
>  				       struct repository *r UNUSED,
>  				       const struct object_id *oid UNUSED,
>  				       struct packed_git *pack,
>  				       off_t offset)
>  {
> +	struct odb_read_stream stream = {
> +		.close = close_istream_pack_non_delta,
> +		.read = read_istream_pack_non_delta,
> +	};