Hi,
On Tue, 10 Mar 2009, Johannes Sixt wrote:
> Shawn O. Pearce schrieb:
> > Johannes Sixt <j.sixt@xxxxxxxxxxxxx> 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?
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
