From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 04 Mar 2026 22:11:31 GMT
Subject: Re: [PATCH v2 08/10] csum-file: introduce `hashfd_ext()`
Message-ID: <xmqqjyvr5jos.fsf@gitster.g>
In-Reply-To: <20260303-pks-upload-pack-write-contention-v2-8-7321830f08fe@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> -static struct hashfile *hashfd_internal(const struct git_hash_algo *algop,
> -					int fd, const char *name,
> -					struct progress *tp,
> -					size_t buffer_len)
> +struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
> +			    int fd, const char *name,
> +			    const struct hashfd_options *opts)

What does _ext stand for?

More seriously, this essentially chooses to pick two parameters
hashfd_internal() takes and put them in a struct, which would give
us a clear upgrade path to add more to the structure without having
to change the function signature.  But what is the criteria used to
choose these two among 5 parameters the original function takes?

Specifically, I am wondering if fd and algop should be part of the
structure, as these would be exactly the same for repeated calls to
this function to write to a single stream.

> +struct hashfd_options {
> +	/*
> +	 * Throughput progress that counts the number of bytes that have been
> +	 * hashed.
> +	 */
> +	struct progress *progress;
> +
> +	/* The length of the buffer that shall be used read read data. */
> +	size_t buffer_len;
> +};
> +
> +struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
> +			    int fd, const char *name,
> +			    const struct hashfd_options *opts);
>  struct hashfile *hashfd(const struct git_hash_algo *algop,
>  			int fd, const char *name);
>  struct hashfile *hashfd_check(const struct git_hash_algo *algop,