Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 3, 2015, 17:45 UTC
Message-ID: <xmqq38155e3s.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150702135309.GA18286@peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> On Wed, Jul 01, 2015 at 01:39:49PM -0700, Junio C Hamano wrote:
>
>> There is a slight complication on sending an empty line without any
>> termination, though ;-)  The reader that calls packet_read() cannot
>> tell such a payload from a flush packet, I think.
>> 
>> *That* may be something we want to document.
>
> Usually flush packets are "0000", and an empty data packet
> is "0004". Or are you talking about some kind of flush inside the
> pkt-data stream?

Neither. At the wire level there is a difference, but the callers of most often used function in pkt-line API, packet_read(), says

	while (1) {
		len = packet_read();
	        if (!len) {
	        	/* flush */
	                break;
		}
	        ... do things on the "len" bytes received ...
		... and then on to the next packet ...
	}

I think the least intrusive change to the caller side would be to teach packet_read() to keep a static and let the callers do this:

	while (1) {
		len = packet_read();
	        if (!len && packet_last_was_flush()) {
	        	/* flush */
	                break;
		}
	        ... do things on the "len" bytes received ...
		... and then on to the next packet ...
	}

even though that looks very ugly.

	len = packet_read(..., &flag);
        if (!len && (flag & PKT_LAST_WAS_FLUSH)) {
        	/* flush */
                ...

might be better.