Re: [PATCH] Update packfile transfer protocol documentation

"Shawn O. Pearce" [off-list ref] writes:

quoted

quoted

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

Junio C Hamano [off-list ref] wrote:

"Shawn O. Pearce" [off-list ref] writes:

quoted

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.

If its pointless, why encourage it?  Why not discourage it with SHOULD NOT?
 

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)

The smart-http series causes fetch-pack to buffer.  :-)

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

Sure, but can't packet_write just return early without write()
if format_packet returned 4 (aka vsnprintf returned 0)?

-- 
Shawn.