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

From: Junio C Hamano <[email-masked]>
Date: Fri, 03 Jul 2015 10:45:59 -0700

Jeff King <[email-masked]> writes:

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