Subject: Re: [PATCHv17 05/11] run_processes_parallel: treat output of children as byte array

From: Junio C Hamano <[email-masked]>
Date: Thu, 25 Feb 2016 10:16:34 -0800

Stefan Beller <[email-masked]> 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?