On 15 Feb 2022, at 20:28, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> +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?
>
>> + 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?
Yeah, this is what I had before but there was discussion about separation of concerns in [1]. But perhaps it's preferable compared to passing a pointer to nr.
1. https://lore.kernel.org/git/CAPig+cTwLhn1GZ_=6s0FXL0z=Q=p1w9ZGK0hAV8wfK9RsQYjnA@xxxxxxxxxxxxxx/
