Re: [PATCH v7 4/4] cat-file: add --batch-command mode

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 16, 2022, 01:28 UTC
Message-ID: <xmqqfsojy50b.fsf@gitster.g>
In-Reply-To: <aebaf7e3fe1d4448037d2caf03f3de393908ff9a.1644972810.git.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

> +static void dispatch_calls(struct batch_options *opt,
> +		struct strbuf *output,
> +		struct expand_data *data,
> +		struct queued_cmd *cmd,
> +		size_t *nr)
> +{
> +	int i;
> +
> +	if (!opt->buffer_output)
> +		die(_("flush is only for --buffer mode"));
> +
> +	for (i = 0; i < *nr; i++) {

If you updated the max number of items *nr to size_t, don't you need to use 'i' with the same type to count up to it?

Show 7 quoted lines

> +		cmd[i].fn(opt, cmd[i].line, output, data);
> +		free(cmd[i].line);
> +	}
> +
> +	*nr = 0;
> +	fflush(stdout);
> +}

Wouldn't it be easier to reason about what the caller/callee are responsible for, if the function signature looked more like:

	static size_t dispatch_calls(struct batch_options *opt,
				     ...
				     struct queued_cmd cmd[], size_t nr)
	{
		size_t i;

		for (i = 0; i < nr; i++)
			... do stuff ...;

		return updated_nr;
	}

and make the caller do

	nr = dispatch_calls(opt, ..., nr);

or if the function *never* leaves anything in the queue when it returns, then

	static void dispatch_calls(struct batch_options *opt,
				     ...
				     struct queued_cmd cmd[], size_t nr)
	{
		size_t i;

		for (i = 0; i < nr; i++)
			... do stuff ...;
	}

and make the caller do

	dispatch_calls(opt, ..., nr);
	nr = 0;

instead of passing a pointer to nr like the posted patch?