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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 24, 2017, 22:38 UTC
Message-ID: <20170324223848.GH31294@aiede.mtv.corp.google.com>
In-Reply-To: <20170324182902.19280-4-sbeller@google.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