Re: [PATCHv15 2/5] run_processes_parallel: add LF when caller is sloppy

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 24, 2016, 20:07 UTC
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:

Show 14 quoted lines

> @@ -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?