From: Stefan Beller <sbeller@google.com>
Date: Thu, 25 Feb 2016 20:35:04 GMT
Subject: Re: [PATCHv17 05/11] run_processes_parallel: treat output of children as byte array
Message-ID: <CAGZ79kaQHk+1k48Yrn_2p9SwB8aJO96jUtL-cA-Gr1PASMkUkQ@mail.gmail.com>
In-Reply-To: <xmqqsi0godtp.fsf@gitster.mtv.corp.google.com>

On Thu, Feb 25, 2016 at 10:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> @@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)
>>                       strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
>>                       strbuf_reset(&pp->children[i].err);
>>               } else {
>> -                     fputs(pp->children[i].err.buf, stderr);
>> +                     strbuf_write(&pp->children[i].err, stderr);
>>                       strbuf_reset(&pp->children[i].err);
>>
>>                       /* Output all other finished child processes */
>> -                     fputs(pp->buffered_output.buf, stderr);
>> +                     strbuf_write(&pp->buffered_output, stderr);
>>                       strbuf_reset(&pp->buffered_output);
>>
>>                       /*
>> diff --git a/strbuf.c b/strbuf.c
>> index 38686ff..71345cd 100644
>> --- a/strbuf.c
>> +++ b/strbuf.c
>> @@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
>>       return cnt;
>>  }
>>
>> +ssize_t strbuf_write(struct strbuf *sb, FILE *f)
>> +{
>> +     return fwrite(sb->buf, 1, sb->len, f);
>> +}
>
> Whenever I see a call to a function that takes size and nmemb
> separately, I get worried about the case where nmemb is zero.
> Hopefully everybody implements such a fwrite() as a no-op?
>
> This may not matter in this patch as no caller checks the return
> value from this function, but shouldn't the callers be a bit more
> careful checking errors in the first place?

If there is no other comment on the series, I plan on sending a patch
to fix this up afterwards to not further collide with the refs backend series.