All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [RFC/PATCH] pkt-line: allow writing of LARGE_PACKET_MAX buffers
Date: Tue, 09 Dec 2014 14:41:51 -0800	[thread overview]
Message-ID: <xmqqa92wla34.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20141209180916.GA26873@peff.net> (Jeff King's message of "Tue, 9 Dec 2014 13:09:16 -0500")

Jeff King <peff@peff.net> writes:

> On Tue, Dec 09, 2014 at 12:49:58PM -0500, Jeff King wrote:
>
>> Another option would be to use a static strbuf. Then we're only wasting
>> heap, and even then only as much as we need (we'd still manually cap it
>> at LARGE_PACKET_MAX since that's what the protocol dictates). This would
>> also make packet_buf_write more efficient (right now it formats into a
>> static buffer, and then copies the result into a strbuf; probably not
>> measurably important, but silly nonetheless).
>
> Below is what that would look like. It's obviously a much more invasive
> change, but I think the result is nice.

Yes, indeed.  Is there any reason why we shouldn't go with this
variant, other than "it touches a bit more lines" that I am not
seeing?

> Let's switch to using a strbuf, with a hard-limit of
> LARGE_PACKET_MAX (which is specified by the protocol).  This
> matches the size of the readers, as of 74543a0 (pkt-line:
> provide a LARGE_PACKET_MAX static buffer, 2013-02-20).
> Versions of git older than that will complain about our
> large packets, but it's really no worse than the current
> behavior. Right now the sender barfs with "impossibly long
> line" trying to send the packet, and afterwards the reader
> will barf with "protocol error: bad line length %d", which
> is arguably better anyway.

Anything older than 1.8.3 is affected by this, but only when the
sending side has to send a large packet.  It is between failing
because the sender cannot send a large packet and failing because
the receiver does not expect such a large packet to come, and either
way the whole operation will fail anyway, so there is no net loss.

  reply	other threads:[~2014-12-09 22:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-09 17:49 [RFC/PATCH] pkt-line: allow writing of LARGE_PACKET_MAX buffers Jeff King
2014-12-09 18:09 ` Jeff King
2014-12-09 22:41   ` Junio C Hamano [this message]
2014-12-10  4:36     ` Michael Blume
2014-12-10  7:34     ` [PATCH v3] " Jeff King
2014-12-10  8:36       ` Eric Sunshine
2014-12-10  9:42         ` Eric Sunshine
2014-12-10  9:49           ` Eric Sunshine
2014-12-10  9:53             ` Jeff King
2014-12-10 10:39               ` [PATCH 0/3] convert read_packed_refs to use strbuf Jeff King
2014-12-10 10:40                 ` [PATCH 1/3] read_packed_refs: use a strbuf for reading lines Jeff King
2014-12-10 10:40                 ` [PATCH 2/3] read_packed_refs: pass strbuf to parse_ref_line Jeff King
2014-12-10 10:40                 ` [PATCH 3/3] read_packed_refs: use skip_prefix instead of static array Jeff King
2014-12-10 17:43                 ` [PATCH 0/3] convert read_packed_refs to use strbuf Junio C Hamano
2014-12-10  9:47         ` [PATCH v4] pkt-line: allow writing of LARGE_PACKET_MAX buffers Jeff King
2014-12-10  9:56           ` Eric Sunshine
2014-12-10 20:14           ` Eric Sunshine
2014-12-10 21:06             ` Jeff King
2014-12-09 19:58 ` [RFC/PATCH] " Johannes Sixt
2014-12-09 20:00   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa92wla34.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.