Re: [PATCH 02/18] streaming: drop the `open()` callback function
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Nov 19, 2025, 09:39 UTC
- Message-ID
- <CAOLa=ZRX+_NO-KqiDDtDeLWTKgwMTFDqfcgZjvOechScy+Rv3w@mail.gmail.com>
- In-Reply-To
- <20251119-b4-pks-odb-read-stream-v1-2-adacf03c2ccf@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 30 quoted lines
> diff --git a/streaming.c b/streaming.c
> index 1fb4b7c1c0..5ce6350123 100644
> --- a/streaming.c
> +++ b/streaming.c
> @@ -14,10 +14,6 @@
> #include "replace-object.h"
> #include "packfile.h"
>
> -typedef int (*open_istream_fn)(struct odb_read_stream *,
> - struct repository *,
> - const struct object_id *,
> - enum object_type *);
> typedef int (*close_istream_fn)(struct odb_read_stream *);
> typedef ssize_t (*read_istream_fn)(struct odb_read_stream *, char *, size_t);
>
> @@ -34,7 +30,6 @@ struct filtered_istream {
> };
>
> struct odb_read_stream {
> - open_istream_fn open;
> close_istream_fn close;
> read_istream_fn read;
>
> @@ -437,21 +432,25 @@ static int istream_source(struct odb_read_stream *st,
>
> switch (oi.whence) {
> case OI_LOOSE:
> - st->open = open_istream_loose;
> + if (open_istream_loose(st, r, oid, type) < 0)
> + break;Earlier we were checking for `if (st->open(st, r, real, type))` so there is a slight change in behavior here.
But both `open_istream_loose()` and `open_istream_pack_non_delta()` return either -1 or 0. So this is okay.
Show 16 quoted lines
> return 0;
> case OI_PACKED:
> - if (!oi.u.packed.is_delta &&
> - repo_settings_get_big_file_threshold(the_repository) < size) {
> - st->u.in_pack.pack = oi.u.packed.pack;
> - st->u.in_pack.pos = oi.u.packed.offset;
> - st->open = open_istream_pack_non_delta;
> - return 0;
> - }
> - /* fallthru */
> - default:
> - st->open = open_istream_incore;
> + if (oi.u.packed.is_delta ||
> + repo_settings_get_big_file_threshold(the_repository) >= size)
> + break;
> +So we switch the branch flow to break the switch early. Makes sense. The patch looks good.
[snip]