Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 10, 2009, 15:07 UTC
Message-ID: <alpine.DEB.1.00.0903101605460.14295@intel-tinevez-2-302>
In-Reply-To: <49B680F7.4040103@viscovery.net>

Hi,

On Tue, 10 Mar 2009, Johannes Sixt wrote:

Show 40 quoted lines

> Shawn O. Pearce schrieb:
> > Johannes Sixt <j.sixt@viscovery.net> wrote:
> >> diff --git a/sideband.c b/sideband.c
> >> index cca3360..a706ac8 100644
> >> --- a/sideband.c
> >> +++ b/sideband.c
> >> @@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
> >>  		switch (band) {
> >>  		case 3:
> >>  			buf[pf] = ' ';
> >> -			buf[pf+1+len] = '\n';
> >> -			safe_write(err, buf, pf+1+len+1);
> >> +			buf[pf+1+len] = '\0';
> >> +			fprintf(stderr, "%s\n", buf);
> > 
> > Can't you instead do:
> > 
> > 	fprintf(stderr, "%.*s\n", buf, pf + len);
> > 
> > like you do...
> > 
> >> @@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
> >>  					memcpy(save, b + brk, sf);
> >>  					b[brk + sf - 1] = b[brk - 1];
> >>  					memcpy(b + brk - 1, suffix, sf);
> >> -					safe_write(err, b, brk + sf);
> >> +					fprintf(stderr, "%.*s", brk + sf, b);
> >>  					memcpy(b + brk, save, sf);
> >>  					len -= brk;
> >>  				} else {
> >>  					int l = brk ? brk : len;
> >> -					safe_write(err, b, l);
> >> +					if (l > 0)
> >> +						fprintf(stderr, "%.*s", l, b);
> > 
> > here?
> 
> I deliberatly avoided "%.*s" in the former hunk (1) because of the posts
> that we had yesterday about potentially misbehaved fprintf in the case
> where the precision is 0;

Didn't that turn out to be a false alarm?

Show 7 quoted lines

> and (2) because it was so easy to avoid it. I don't think we need 
> ultimate performance in this case, and I also consider the plain "%s\n" 
> more readable.
> 
> That said, the second hunk is really only the minimal change and I'd 
> like to rewrite it to get rid of the memcpy stuff. It is really not 
> needed once fprintf is in the game. But that's a separate patch.

I think, indeed, that you can avoid the memcpy() by using %.*s. The private buffer is only used to make sure that the text is written in one go anyway (i.e. that two sidebands messages are not written to the same line because they use multiple calls to fprintf()/fwrite() per line), right?

Ciao, Dscho