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

Hi,

On Tue, 10 Mar 2009, Johannes Sixt wrote:

Shawn O. Pearce schrieb:

quoted

Johannes Sixt [off-list ref] wrote:

quoted

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...

quoted

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

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