Re: [PATCH v4] Refactor recv_sideband()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 28, 2016, 22:33 UTC
Message-ID: <xmqqfurx6j16.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.LFD.2.20.1606281726330.24439@knanqh.ubzr>

Nicolas Pitre <nico@fluxnic.net> writes:

Show 40 quoted lines

>> The basic structure of the code (without the "SQUASH" we discussed)
>> looks like this:
>> 
>> 	strbuf_addf(&outbuf, "%s", PREFIX);
>> 	while (retval == 0) {
>> 		len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 0);
>> 		...
>> 		band = buf[0] & 0xff;
>> 		switch (band) {
>> 		case 3:
>> 			... /* emergency exit */
>> 		case 2:
>> 			while ((brk = strpbrk(b, "\n\r"))) {
>> 				int linelen = brk - b;
>> 
>> 				if (linelen > 0) {
>> 					strbuf_addf(&outbuf, "%.*s%s%c",
>> 						    linelen, b, suffix, *brk);
>> 				} else {
>> 					strbuf_addf(&outbuf, "%c", *brk);
>> 				}
>> 				fprintf(stderr, "%.*s", (int)outbuf.len,
>> 					outbuf.buf);
>> 				strbuf_reset(&outbuf);
>> 				strbuf_addf(&outbuf, "%s", PREFIX);
>> 				b = brk + 1;
>> 			}
>> 			if (*b)
>> 				strbuf_addf(&outbuf, "%s", b);
>> 			break;
>> 		...
>> 		}
>> 	}
>> 
>> 	if (outbuf.len > 0)
>> 		fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
>>  ...
> That won't work. If at this point there is the beginning of a partial 
> line queued in the buffer from the previous round waiting to see its 
> line break, you just deleted the beginning of that line.

Ahh, OK, a single logical line split into two and sent in two packets--the first one would not result in any output (just does "if (*b) strbuf_addf(...)" to buffer it) and then the second one finally finds a LF. Yeah, that won't work if we cleared.

But then my observation still holds, no?

	while () {
        	read();
                switch () {
                case 2:
                	while ((brk = strpbfk())) {
                        	/* we have LF! we'll say something! */
                                strbuf_splice(&outbuf, 0, 0,
                                	      PREFIX, strlen(PREFIX));
                                strbuf_addf(&outbuf, ...);
                                fwrite(...);
				b = brk + 1;
			}
                        if (*b)
                        	strbuf_addstr(&outbuf, b);
			break;
		}
	}

        if (outbuf.len) {
        	/* we still have something to say */
                strbuf_splice(&outbuf, 0, 0, PREFIX,
        	strlen(PREFIX));
                fwrite(...);
	}