Re: [PATCH v4] Refactor recv_sideband()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 28, 2016, 22:47 UTC
Message-ID: <xmqq8txp6icn.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqfurx6j16.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 9 quoted lines

> But then my observation still holds, no?
>
> ...
>         if (outbuf.len) {
>         	/* we still have something to say */
>                 strbuf_splice(&outbuf, 0, 0, PREFIX,
>         	strlen(PREFIX));
>                 fwrite(...);
> 	}

I guess these two are more or less equivalent.

I view "outbuf" more as "holdbuf", i.e. the payload that has been received and yet to be shown, and from that point of view, having and keeping PREFIX which is not something we received in there while looping to grab more input makes me feel dirty. IOW, I consider "empty" is the correct base state of the hold buffer before anything happens.

I understand that you view "outbuf" as what has been prepared to be shown but not ready to be shown due to lack of LF, and from that point of view, the base state of the out buffer before anything happens could be "it has PREFIX and nothing else".

It's just that if you take the latter, then the conditional after the loop exits (i.e. the last transmission was an incomplete line) cannot be "is outbuf empty?", as your base state is "has PREFIX and can never be empty". I was working back from that if statement.