Re: [PATCH v4 1/5] run-command: add duplicate_output_fn to run_processes_parallel_opts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Calvin Wan <calvinwan@xxxxxxxxxx> writes:

> @@ -1680,8 +1683,14 @@ static void pp_buffer_stderr(struct parallel_processes *pp,
>  	for (size_t i = 0; i < opts->processes; i++) {
>  		if (pp->children[i].state == GIT_CP_WORKING &&
>  		    pp->pfd[i].revents & (POLLIN | POLLHUP)) {
> -			int n = strbuf_read_once(&pp->children[i].err,
> -						 pp->children[i].process.err, 0);
> +			struct strbuf buf = STRBUF_INIT;
> +			int n = strbuf_read_once(&buf, pp->children[i].process.err, 0);
> +			strbuf_addbuf(&pp->children[i].err, &buf);
> +			if (opts->duplicate_output)
> +				opts->duplicate_output(&buf, &pp->children[i].err,
> +					  opts->data,
> +					  pp->children[i].data);
> +			strbuf_release(&buf);
>  			if (n == 0) {
>  				close(pp->children[i].process.err);
>  				pp->children[i].state = GIT_CP_WAIT_CLEANUP;

A common pattern is that optional behavior does not impose additional
costs on the non-optional part. Here, we unconditionally do a
strbuf_addbuf() even though we don't use the result in the "else" case.

So this might be more idiomatically written as:

        int n = strbuf_read_once(&pp->children[i].err,
        			 pp->children[i].process.err, 0);
 +      if (opts->duplicate_output) {
 +          struct strbuf buf = STRBUF_INIT;
 +          strbuf_addbuf(&buf, &pp->children[i].err);
 +        	opts->duplicate_output(&buf, &pp->children[i].err,
 +        		  opts->data,
 +        		  pp->children[i].data);
 +          strbuf_release(&buf);
 +      }

which also has the nice benefit of not touching the strbuf_read_once()
line.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux