Re: [PATCH v4] Refactor recv_sideband()

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

 



Nicolas Pitre <nico@xxxxxxxxxxx> writes:

> To make it clearer, here's a patch on top of pu that fixes all the 
> issues I think are remaining. All tests pass now.
>
> diff --git a/sideband.c b/sideband.c
> index 36a032f..0e6c6df 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -23,10 +23,8 @@ int recv_sideband(const char *me, int in_stream, int out)
>  	const char *term, *suffix;
>  	char buf[LARGE_PACKET_MAX + 1];
>  	struct strbuf outbuf = STRBUF_INIT;
> -	const char *b, *brk;
>  	int retval = 0;
>  
> -	strbuf_addf(&outbuf, "%s", PREFIX);
>  	term = getenv("TERM");
>  	if (isatty(2) && term && strcmp(term, "dumb"))
>  		suffix = ANSI_SUFFIX;
> @@ -34,14 +32,15 @@ int recv_sideband(const char *me, int in_stream, int out)
>  		suffix = DUMB_SUFFIX;
>  
>  	while (!retval) {
> +		const char *b, *brk;
>  		int band, len;
>  		len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 0);
>  		if (len == 0)
>  			break;
>  		if (len < 1) {
>  			strbuf_addf(&outbuf,
> -				    "\n%s: protocol error: no band designator\n",
> -				    me);
> +				    "%s%s: protocol error: no band designator",
> +				    outbuf.len ? "\n" : "", me);

OK, because you no longer put PREFIX in outbuf, outbuf.len becomes
an easy way to tell if we have anything accumulated, and if there
already is something, we separate our message from it with a LF.

I like the simplicity and clarity of the logic.

> @@ -50,7 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out)
>  		len--;
>  		switch (band) {
>  		case 3:
> -			strbuf_addf(&outbuf, "\n%s%s\n", PREFIX, buf + 1);
> +			strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "",
> +				    PREFIX, buf + 1);

Likewise for all other changes.

> -	if (outbuf.len)
> +	if (outbuf.len) {
> +		strbuf_addf(&outbuf, "\n");
>  		fwrite(outbuf.buf, 1, outbuf.len, stderr);
> +	}

... and this does make sense.

Lukas, can you see what is in 'pu' after I push out today's
integration result in several hours and tell us if you like the
result of the SQUASH??? change?

Thanks.

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



[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]