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

[Junio C Hamano <gitster@pobox.com>]

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

> +/* Loop through each queued_cmd, dispatch the function, free the
> + * memory associated with line so the cmd array can be reused.
> + * Callers must set nr back to 0 in order to reuse the cmd array.
> + */

Style.

	/*
	 * Our multi-line comments look like this;
	 * slash-asterisk on the opening line and
	 * asterisk-slash on the closing line sit
	 * on their own lines.
	 */

I am not sure if that is an accurate description.  The caller does
not have to reuse the cmd array.  The only thing they have to know
about this function is that the .line member is freed after the
function is done with it, so they do not have to free the member
themselves.

As it seems that this code structure and division of responsibility
between the caller and the callee is confusing even to the author of
this code, it may make sense to make the caller responsible for
freeing.

Then the caller becomes

> +		if (!strcmp(cmd->name, "flush")) {
> +			dispatch_calls(opt, output, data, queued_cmd, nr);

			for (i = 0; i < nr; i++)
				free(queued_cmd[i].line);

> +			nr = 0;
> +			continue;
> +		}

which is not too bad.  And then we'd free the array itself at the
end ...

> ...
> +		call.line = xstrdup_or_null(p);
> +		queued_cmd[nr++] = call;
> +	}
> +
> +	if (opt->buffer_output &&
> +	    nr &&
> +	    !git_env_bool("GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT", 0))
> +		dispatch_calls(opt, output, data, queued_cmd, nr);
> +
> +	free(queued_cmd);

... which may be easier to see what is going on.

Thanks.