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

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