Re: [PATCH] Update packfile transfer protocol documentation

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2009, 01:07 UTC
Message-ID: <7vljinnllj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20091104005614.GD10505@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

Show 10 quoted lines

>> > +A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
>> > +is a special case and MUST be handled differently than an empty
>> > +pkt-line ("0004").
>> 
>> ...especially that this sentence makes it sound as if it is perfectly
>> normal to send "0004" for "an empty line" (and I've always thought that is
>> Ok), I am quite puzzled by that "SHOULD NOT".
>
> I don't think we ever send an empty packet.  If we have no data to
> send, why the hell did we create the packet header?

Oh, I do not disagree that it is pointless, but the example that followed the part we are discussing also had "0004". I think it is Ok to allow it.

The original intent of packet_flush() was that everything else could be kept buffered in-core without going to write() until packet_flush() is called. The protocol was defined in a way that we won't wait for listening a response from the other end to an earlier message we "sent" with packet_write() but haven't called packet_flush() yet hence could still be in our buffer. We still have this comment:

    /*
     * If we buffered things up above (we don't, but we should),
     * we'd flush it here
     */
    void packet_flush(int fd)

And once we start buffering, allowing "0004" packet_write() wouldn't even be a problem; it can be optimized out in the buffering layer.