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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2016, 18:16 UTC
Message-ID: <xmqqsi0godtp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1456369618-18127-6-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 26 quoted lines

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