summaryrefslogtreecommitdiff
path: root/pkt-line.c
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2013-02-23 17:31:34 -0500
committerJunio C Hamano <gitster@pobox.com>2013-02-24 00:14:15 -0800
commit4981fe750b1fc58bfdf5b9ca9843f4f505b9bb4d (patch)
tree0dba124f33bf0f6e46bbc1a5621efe70150b9941 /pkt-line.c
parent74543a0423c96130b3b07946c20b10735c3b5b15 (diff)
downloadgit-4981fe750b1fc58bfdf5b9ca9843f4f505b9bb4d.tar.gz
pkt-line: share buffer/descriptor reading implementation
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: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'pkt-line.c')
-rw-r--r--pkt-line.c76
1 files changed, 38 insertions, 38 deletions
diff --git a/pkt-line.c b/pkt-line.c
index 55fb688899..70f19501d0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -104,12 +104,28 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
strbuf_add(buf, buffer, n);
}
-static int safe_read(int fd, void *buffer, unsigned size, int options)
+static int get_packet_data(int fd, char **src_buf, size_t *src_size,
+ void *dst, unsigned size, int options)
{
- ssize_t ret = read_in_full(fd, buffer, size);
- if (ret < 0)
- die_errno("read error");
- else if (ret < size) {
+ ssize_t ret;
+
+ if (fd >= 0 && src_buf && *src_buf)
+ die("BUG: multiple sources given to packet_read");
+
+ /* Read up to "size" bytes from our source, whatever it is. */
+ if (src_buf && *src_buf) {
+ ret = size < *src_size ? size : *src_size;
+ memcpy(dst, *src_buf, ret);
+ *src_buf += ret;
+ *src_size -= ret;
+ } else {
+ ret = read_in_full(fd, dst, size);
+ if (ret < 0)
+ die_errno("read error");
+ }
+
+ /* And complain if we didn't get enough bytes to satisfy the read. */
+ if (ret < size) {
if (options & PACKET_READ_GENTLE_ON_EOF)
return -1;
@@ -144,12 +160,13 @@ static int packet_length(const char *linelen)
return len;
}
-int packet_read(int fd, char *buffer, unsigned size, int options)
+int packet_read(int fd, char **src_buf, size_t *src_len,
+ char *buffer, unsigned size, int options)
{
int len, ret;
char linelen[4];
- ret = safe_read(fd, linelen, 4, options);
+ ret = get_packet_data(fd, src_buf, src_len, linelen, 4, options);
if (ret < 0)
return ret;
len = packet_length(linelen);
@@ -162,7 +179,7 @@ int packet_read(int fd, char *buffer, unsigned size, int options)
len -= 4;
if (len >= size)
die("protocol error: bad line length %d", len);
- ret = safe_read(fd, buffer, len, options);
+ ret = get_packet_data(fd, src_buf, src_len, buffer, len, options);
if (ret < 0)
return ret;
@@ -175,41 +192,24 @@ int packet_read(int fd, char *buffer, unsigned size, int options)
return len;
}
-char *packet_read_line(int fd, int *len_p)
+static char *packet_read_line_generic(int fd,
+ char **src, size_t *src_len,
+ int *dst_len)
{
- int len = packet_read(fd, packet_buffer, sizeof(packet_buffer),
+ int len = packet_read(fd, src, src_len,
+ packet_buffer, sizeof(packet_buffer),
PACKET_READ_CHOMP_NEWLINE);
- if (len_p)
- *len_p = len;
+ if (dst_len)
+ *dst_len = len;
return len ? packet_buffer : NULL;
}
-int packet_get_line(struct strbuf *out,
- char **src_buf, size_t *src_len)
+char *packet_read_line(int fd, int *len_p)
{
- int len;
-
- if (*src_len < 4)
- return -1;
- len = packet_length(*src_buf);
- if (len < 0)
- return -1;
- if (!len) {
- *src_buf += 4;
- *src_len -= 4;
- packet_trace("0000", 4, 0);
- return 0;
- }
- if (*src_len < len)
- return -2;
-
- *src_buf += 4;
- *src_len -= 4;
- len -= 4;
+ return packet_read_line_generic(fd, NULL, NULL, len_p);
+}
- strbuf_add(out, *src_buf, len);
- *src_buf += len;
- *src_len -= len;
- packet_trace(out->buf, out->len, 0);
- return len;
+char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)
+{
+ return packet_read_line_generic(-1, src, src_len, dst_len);
}