Re: [PATCH 3/7] submodule.c: convert is_submodule_modified to use strbuf_getwholeline

[Jonathan Nieder <jrnieder@gmail.com>]

Stefan Beller wrote:

> Instead of implementing line reading yet again, make use of our beautiful
> library function to read one line.  By using strbuf_getwholeline instead
> of strbuf_read, we avoid having to allocate memory for the entire child
> process output at once.  That is, we limit maximum memory usage.

It also overlaps work a little better.

> Once we know all information that we care about, we can terminate
> the child early. In that case we do not care about its exit code as well.

Should this say something about SIGPIPE?

[...]
> +++ b/submodule.c
[...]
> @@ -1072,28 +1072,27 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
[...]
> +			/*
> +			 * We're not interested in any further information from
> +			 * the child any more, no output nor its exit code.
> +			 */

language nit: s/, no output/: neither output/

[...]
> -	if (finish_command(&cp))
> +	if (finish_command(&cp) && !ignore_cp_exit_code)

finish_command complains if the child dies of SIGTERM:

		error: status died of signal 15

wait_or_whine(cp->pid, cp->argv[0], 1) doesn't do that but is meant for
signal handling.  Maybe we should rely on SIGPIPE instead (which
wait_or_whine always silences) and avoid the kill() call.

Can there be a test for this case (i.e. having lots of untracked files
in the submodule so the child process fills its pipe buffer and has to
exit early)?

Thanks,
Jonathan