On Wed, Feb 24, 2016 at 12:07 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> 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?
What is the right thing? I asked myself and obviously it is treating the
child->err the same in both cases of checking for a trailing LF and
when outputting.
But what is the right way to look at child->err? I would argue that
we should allow for children to have a NUL in its output stream and
replay their output as literal as possible.
i.e. my conclusion is to replace the fputs by fwrite as opposed to
using strlen to determine the length of string output.
Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html