From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 24 Feb 2016 20:07:37 GMT
Subject: Re: [PATCHv15 2/5] run_processes_parallel: add LF when caller is sloppy
Message-ID: <xmqqa8mpub1y.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1456284017-26141-3-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> @@ -1095,9 +1098,11 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)
>  static void pp_output(struct parallel_processes *pp)
>  {
>  	int i = pp->output_owner;
> -	if (pp->children[i].state == GIT_CP_WORKING &&
> -	    pp->children[i].err.len) {
> +	size_t len = pp->children[i].err.len;
> +	if (pp->children[i].state == GIT_CP_WORKING && len) {
>  		fputs(pp->children[i].err.buf, stderr);
> +		pp->ended_with_newline =
> +			(pp->children[i].err.buf[len - 1] == '\n');
>  		strbuf_reset(&pp->children[i].err);
>  	}
>  }

The child->err thing is treated as a counted byte array when the
code determines where the end of the buffer is, but is treated as a
nul-terminated string when it is output with fputs().

The inconsistency may not hurt as long as (1) the producers of the
message will never stuff a NUL in the middle, and (2) strbuf always
has the guard NUL after its contents.  Even though we know that the
latter will hold true for the foreseeable future, it also is easy to
do the right thing here, too, so why not?