Skip to content
  • Jeff King's avatar
    pkt-line: share buffer/descriptor reading implementation · 4981fe75
    Jeff King authored
    
    
    The packet_read function reads from a descriptor. The
    packet_get_line function is similar, but reads from an
    in-memory buffer, and uses a completely separate
    implementation. This patch teaches the generic packet_read
    function to accept either source, and we can do away with
    packet_get_line's implementation.
    
    There are two other differences to account for between the
    old and new functions. The first is that we used to read
    into a strbuf, but now read into a fixed size buffer. The
    only two callers are fine with that, and in fact it
    simplifies their code, since they can use the same
    static-buffer interface as the rest of the packet_read_line
    callers (and we provide a similar convenience wrapper for
    reading from a buffer rather than a descriptor).
    
    This is technically an externally-visible behavior change in
    that we used to accept arbitrary sized packets up to 65532
    bytes, and now cap out at LARGE_PACKET_MAX, 65520. In
    practice this doesn't matter, as we use it only for parsing
    smart-http headers (of which there is exactly one defined,
    and it is small and fixed-size). And any extension headers
    would be breaking the protocol to go over LARGE_PACKET_MAX
    anyway.
    
    The other difference is that packet_get_line would return
    on error rather than dying. However, both callers of
    packet_get_line are actually improved by dying.
    
    The first caller does its own error checking, but we can
    drop that; as a result, we'll actually get more specific
    reporting about protocol breakage when packet_read dies
    internally. The only downside is that packet_read will not
    print the smart-http URL that failed, but that's not a big
    deal; anybody not debugging can already see the remote's URL
    already, and anybody debugging would want to run with
    GIT_CURL_VERBOSE anyway to see way more information.
    
    The second caller, which is just trying to skip past any
    extra smart-http headers (of which there are none defined,
    but which we allow to keep room for future expansion), did
    not error check at all. As a result, it would treat an error
    just like a flush packet. The resulting mess would generally
    cause an error later in get_remote_heads, but now we get
    error reporting much closer to the source of the problem.
    
    Brown-paper-bag-fixes-by: default avatarRamsay Jones <ramsay@ramsay1.demon.co.uk>
    Signed-off-by: default avatarJeff King <peff@peff.net>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    4981fe75