pkt-line: provide a LARGE_PACKET_MAX static buffer

Most of the callers of packet_read_line just read into a
static 1000-byte buffer (callers which handle arbitrary
binary data already use LARGE_PACKET_MAX). This works fine
in practice, because:

1. The only variable-sized data in these lines is a ref
name, and refs tend to be a lot shorter than 1000
characters.

2. When sending ref lines, git-core always limits itself
to 1000 byte packets.

However, the only limit given in the protocol specification
in Documentation/technical/protocol-common.txt is
LARGE_PACKET_MAX; the 1000 byte limit is mentioned only in
pack-protocol.txt, and then only describing what we write,
not as a specific limit for readers.

This patch lets us bump the 1000-byte limit to
LARGE_PACKET_MAX. Even though git-core will never write a
packet where this makes a difference, there are two good
reasons to do this:

1. Other git implementations may have followed
protocol-common.txt and used a larger maximum size. We
don't bump into it in practice because it would involve
very long ref names.

2. We may want to increase the 1000-byte limit one day.
Since packets are transferred before any capabilities,
it's difficult to do this in a backwards-compatible
way. But if we bump the size of buffer the readers can
handle, eventually older versions of git will be
obsolete enough that we can justify bumping the
writers, as well. We don't have plans to do this
anytime soon, but there is no reason not to start the
clock ticking now.

Just bumping all of the reading bufs to LARGE_PACKET_MAX
would waste memory. Instead, since most readers just read
into a temporary buffer anyway, let's provide a single
static buffer that all callers can use. We can further wrap
this detail away by having the packet_read_line wrapper just
use the buffer transparently and return a pointer to the
static storage.  That covers most of the cases, and the
remaining ones already read into their own LARGE_PACKET_MAX
buffers.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/archive.c b/builtin/archive.c
index d381ac4147ff6dd3019d223eda2c3256884f2adf..49178f159e246c9a99805d806fa5f096337e8d2f 100644 (file)
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -27,8 +27,8 @@ static int run_remote_archiver(int argc, const char **argv,
                               const char *remote, const char *exec,
                               const char *name_hint)
 {
                               const char *remote, const char *exec,
                               const char *name_hint)
 {

-       char buf[LARGE_PACKET_MAX];
-       int fd[2], i, len, rv;
+       char *buf;
+       int fd[2], i, rv;

        struct transport *transport;
        struct remote *_remote;
 
        struct transport *transport;
        struct remote *_remote;
 


@@ -53,19 +53,18 @@ static int run_remote_archiver(int argc, const char **argv,
                packet_write(fd[1], "argument %s\n", argv[i]);
        packet_flush(fd[1]);
 
                packet_write(fd[1], "argument %s\n", argv[i]);
        packet_flush(fd[1]);
 

-       len = packet_read_line(fd[0], buf, sizeof(buf));
-       if (!len)
+       buf = packet_read_line(fd[0], NULL);
+       if (!buf)

                die(_("git archive: expected ACK/NAK, got EOF"));
        if (strcmp(buf, "ACK")) {
                die(_("git archive: expected ACK/NAK, got EOF"));
        if (strcmp(buf, "ACK")) {

-               if (len > 5 && !prefixcmp(buf, "NACK "))
+               if (!prefixcmp(buf, "NACK "))

                        die(_("git archive: NACK %s"), buf + 5);
                        die(_("git archive: NACK %s"), buf + 5);

-               if (len > 4 && !prefixcmp(buf, "ERR "))
+               if (!prefixcmp(buf, "ERR "))

                        die(_("remote error: %s"), buf + 4);
                die(_("git archive: protocol error"));
        }
 
                        die(_("remote error: %s"), buf + 4);
                die(_("git archive: protocol error"));
        }
 

-       len = packet_read_line(fd[0], buf, sizeof(buf));
-       if (len)
+       if (packet_read_line(fd[0], NULL))

                die(_("git archive: expected a flush"));
 
        /* Now, start reading from fd[0] and spit it out to stdout */
                die(_("git archive: expected a flush"));
 
        /* Now, start reading from fd[0] and spit it out to stdout */

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index f73664f433c9b602515fe28456da8c86ef0fe4b8..c21cc2c778ae35229f49cddfc55921c0ff7fb8b4 100644 (file)
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -100,12 +100,11 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
                        /* in stateless RPC mode we use pkt-line to read
                         * from stdin, until we get a flush packet
                         */
                        /* in stateless RPC mode we use pkt-line to read
                         * from stdin, until we get a flush packet
                         */

-                       static char line[1000];

                        for (;;) {
                        for (;;) {

-                               int n = packet_read_line(0, line, sizeof(line));
-                               if (!n)
+                               char *line = packet_read_line(0, NULL);
+                               if (!line)

                                        break;
                                        break;

-                               string_list_append(&sought, xmemdupz(line, n));
+                               string_list_append(&sought, xstrdup(line));

                        }
                }
                else {
                        }
                }
                else {

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6679e636c721ec28ac818d5584813dbe8ddea785..ccebd74f166f5d02e8e6a0bb6033d741f8580f7b 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -754,14 +754,14 @@ static struct command *read_head_info(void)
        struct command *commands = NULL;
        struct command **p = &commands;
        for (;;) {
        struct command *commands = NULL;
        struct command **p = &commands;
        for (;;) {

-               static char line[1000];
+               char *line;

                unsigned char old_sha1[20], new_sha1[20];
                struct command *cmd;
                char *refname;
                int len, reflen;
 
                unsigned char old_sha1[20], new_sha1[20];
                struct command *cmd;
                char *refname;
                int len, reflen;
 

-               len = packet_read_line(0, line, sizeof(line));
-               if (!len)
+               line = packet_read_line(0, &len);
+               if (!line)

                        break;
                if (len < 83 ||
                    line[40] != ' ' ||
                        break;
                if (len < 83 ||
                    line[40] != ' ' ||

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index d90f0aba44fdac817ac48f4329d72e65f1e8f663..af2da35e7d05f4de2d7a77a708ee3cf6f490266e 100644 (file)
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -21,8 +21,6 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
 {
        struct argv_array sent_argv = ARGV_ARRAY_INIT;
        const char *arg_cmd = "argument ";
 {
        struct argv_array sent_argv = ARGV_ARRAY_INIT;
        const char *arg_cmd = "argument ";

-       char buf[4096];
-       int len;

 
        if (argc != 2)
                usage(upload_archive_usage);
 
        if (argc != 2)
                usage(upload_archive_usage);


@@ -33,9 +31,8 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
        /* put received options in sent_argv[] */
        argv_array_push(&sent_argv, "git-upload-archive");
        for (;;) {
        /* put received options in sent_argv[] */
        argv_array_push(&sent_argv, "git-upload-archive");
        for (;;) {

-               /* This will die if not enough free space in buf */
-               len = packet_read_line(0, buf, sizeof(buf));
-               if (len == 0)
+               char *buf = packet_read_line(0, NULL);
+               if (!buf)

                        break;  /* got a flush */
                if (sent_argv.argc > MAX_ARGS)
                        die("Too many options (>%d)", MAX_ARGS - 1);
                        break;  /* got a flush */
                if (sent_argv.argc > MAX_ARGS)
                        die("Too many options (>%d)", MAX_ARGS - 1);

diff --git a/connect.c b/connect.c
index fe8eb01ae21c15f1013fd398825aeb20f9630336..611ffb4419f402d6f1f2bc08a91a7ed99aab35e2 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -72,11 +72,11 @@ struct ref **get_remote_heads(int in, struct ref **list,
        for (;;) {
                struct ref *ref;
                unsigned char old_sha1[20];
        for (;;) {
                struct ref *ref;
                unsigned char old_sha1[20];

-               static char buffer[1000];

                char *name;
                int len, name_len;
                char *name;
                int len, name_len;

+               char *buffer = packet_buffer;

 
 

-               len = packet_read(in, buffer, sizeof(buffer),
+               len = packet_read(in, packet_buffer, sizeof(packet_buffer),

                                  PACKET_READ_GENTLE_ON_EOF |
                                  PACKET_READ_CHOMP_NEWLINE);
                if (len < 0)
                                  PACKET_READ_GENTLE_ON_EOF |
                                  PACKET_READ_CHOMP_NEWLINE);
                if (len < 0)

diff --git a/daemon.c b/daemon.c
index 4f5cd615589901c94c1439c9bff3e870d3d4af32..3f70e79b8e6344512c84b026f857329bd459fcf1 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -604,7 +604,7 @@ static void parse_host_arg(char *extra_args, int buflen)
 
 static int execute(void)
 {
 
 static int execute(void)
 {

-       static char line[1000];
+       char *line = packet_buffer;

        int pktlen, len, i;
        char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
 
        int pktlen, len, i;
        char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
 


@@ -612,7 +612,7 @@ static int execute(void)
                loginfo("Connection from %s:%s", addr, port);
 
        alarm(init_timeout ? init_timeout : timeout);
                loginfo("Connection from %s:%s", addr, port);
 
        alarm(init_timeout ? init_timeout : timeout);

-       pktlen = packet_read(0, line, sizeof(line), 0);
+       pktlen = packet_read(0, packet_buffer, sizeof(packet_buffer), 0);

        alarm(0);
 
        len = strlen(line);
        alarm(0);
 
        len = strlen(line);

diff --git a/fetch-pack.c b/fetch-pack.c
index f830db224bd6e05534de232331034b204286af5b..66ff9add89a43e7d22dc6761371fb4532b8b7123 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -172,8 +172,8 @@ static void consume_shallow_list(struct fetch_pack_args *args, int fd)
                 * shallow and unshallow commands every time there
                 * is a block of have lines exchanged.
                 */
                 * shallow and unshallow commands every time there
                 * is a block of have lines exchanged.
                 */

-               char line[1000];
-               while (packet_read_line(fd, line, sizeof(line))) {
+               char *line;
+               while ((line = packet_read_line(fd, NULL))) {

                        if (!prefixcmp(line, "shallow "))
                                continue;
                        if (!prefixcmp(line, "unshallow "))
                        if (!prefixcmp(line, "shallow "))
                                continue;
                        if (!prefixcmp(line, "unshallow "))


@@ -215,8 +215,8 @@ static int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
 
 static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 {
 
 static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 {

-       static char line[1000];
-       int len = packet_read_line(fd, line, sizeof(line));
+       int len;
+       char *line = packet_read_line(fd, &len);

 
        if (!len)
                die("git fetch-pack: expected ACK/NAK, got EOF");
 
        if (!len)
                die("git fetch-pack: expected ACK/NAK, got EOF");


@@ -346,11 +346,11 @@ static int find_common(struct fetch_pack_args *args,
        state_len = req_buf.len;
 
        if (args->depth > 0) {
        state_len = req_buf.len;
 
        if (args->depth > 0) {

-               char line[1024];
+               char *line;

                unsigned char sha1[20];
 
                send_request(args, fd[1], &req_buf);
                unsigned char sha1[20];
 
                send_request(args, fd[1], &req_buf);

-               while (packet_read_line(fd[0], line, sizeof(line))) {
+               while ((line = packet_read_line(fd[0], NULL))) {

                        if (!prefixcmp(line, "shallow ")) {
                                if (get_sha1_hex(line + 8, sha1))
                                        die("invalid shallow line: %s", line);
                        if (!prefixcmp(line, "shallow ")) {
                                if (get_sha1_hex(line + 8, sha1))
                                        die("invalid shallow line: %s", line);

diff --git a/pkt-line.c b/pkt-line.c
index dc11c407cd8513d423356d16c1b5b0f929264a39..55fb688899333363967f4cd7418560e28e352c26 100644 (file)
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "pkt-line.h"
 
 #include "cache.h"
 #include "pkt-line.h"
 

+char packet_buffer[LARGE_PACKET_MAX];

 static const char *packet_trace_prefix = "git";
 static const char trace_key[] = "GIT_TRACE_PACKET";
 
 static const char *packet_trace_prefix = "git";
 static const char trace_key[] = "GIT_TRACE_PACKET";
 


@@ -174,9 +175,13 @@ int packet_read(int fd, char *buffer, unsigned size, int options)
        return len;
 }
 
        return len;
 }
 

-int packet_read_line(int fd, char *buffer, unsigned size)
+char *packet_read_line(int fd, int *len_p)

 {
 {

-       return packet_read(fd, buffer, size, PACKET_READ_CHOMP_NEWLINE);
+       int len = packet_read(fd, packet_buffer, sizeof(packet_buffer),
+                             PACKET_READ_CHOMP_NEWLINE);
+       if (len_p)
+               *len_p = len;
+       return len ? packet_buffer : NULL;

 }
 
 int packet_get_line(struct strbuf *out,
 }
 
 int packet_get_line(struct strbuf *out,

diff --git a/pkt-line.h b/pkt-line.h
index 6927ea521b5f79ca14472859e397113ec19ce093..fa93e32071b2add9eb03ca90ae1632f59b6feef2 100644 (file)
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -54,12 +54,17 @@ int packet_read(int fd, char *buffer, unsigned size, int options);
 
 /*
  * Convenience wrapper for packet_read that is not gentle, and sets the
 
 /*
  * Convenience wrapper for packet_read that is not gentle, and sets the

- * CHOMP_NEWLINE option.
+ * CHOMP_NEWLINE option. The return value is NULL for a flush packet,
+ * and otherwise points to a static buffer (that may be overwritten by
+ * subsequent calls). If the size parameter is not NULL, the length of the
+ * packet is written to it.

  */
  */

-int packet_read_line(int fd, char *buffer, unsigned size);
+char *packet_read_line(int fd, int *size);
+

 
 #define DEFAULT_PACKET_MAX 1000
 #define LARGE_PACKET_MAX 65520
 
 #define DEFAULT_PACKET_MAX 1000
 #define LARGE_PACKET_MAX 65520

+extern char packet_buffer[LARGE_PACKET_MAX];

 
 int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
 
 
 int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
 

diff --git a/send-pack.c b/send-pack.c
index 8c230bf6c925ba6bd3617af62f44cfea8dc09a7b..7d172ef37f7b185f9586ce9015a08481d0bd259c 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -106,9 +106,8 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 static int receive_status(int in, struct ref *refs)
 {
        struct ref *hint;
 static int receive_status(int in, struct ref *refs)
 {
        struct ref *hint;

-       char line[1000];

        int ret = 0;
        int ret = 0;

-       int len = packet_read_line(in, line, sizeof(line));
+       char *line = packet_read_line(in, NULL);

        if (prefixcmp(line, "unpack "))
                return error("did not receive remote status");
        if (strcmp(line, "unpack ok")) {
        if (prefixcmp(line, "unpack "))
                return error("did not receive remote status");
        if (strcmp(line, "unpack ok")) {


@@ -119,8 +118,8 @@ static int receive_status(int in, struct ref *refs)
        while (1) {
                char *refname;
                char *msg;
        while (1) {
                char *refname;
                char *msg;

-               len = packet_read_line(in, line, sizeof(line));
-               if (!len)
+               line = packet_read_line(in, NULL);
+               if (!line)

                        break;
                if (prefixcmp(line, "ok ") && prefixcmp(line, "ng ")) {
                        error("invalid ref status from remote: %s", line);
                        break;
                if (prefixcmp(line, "ok ") && prefixcmp(line, "ng ")) {
                        error("invalid ref status from remote: %s", line);

diff --git a/upload-pack.c b/upload-pack.c
index 6e6d16687640e561cbaa745a2d851a83f3630681..98ddb6958180033c0936884d5dd040148b681950 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -408,7 +408,6 @@ static int ok_to_give_up(void)
 
 static int get_common_commits(void)
 {
 
 static int get_common_commits(void)
 {

-       static char line[1000];

        unsigned char sha1[20];
        char last_hex[41];
        int got_common = 0;
        unsigned char sha1[20];
        char last_hex[41];
        int got_common = 0;


@@ -418,10 +417,10 @@ static int get_common_commits(void)
        save_commit_buffer = 0;
 
        for (;;) {
        save_commit_buffer = 0;
 
        for (;;) {

-               int len = packet_read_line(0, line, sizeof(line));
+               char *line = packet_read_line(0, NULL);

                reset_timeout();
 
                reset_timeout();
 

-               if (!len) {
+               if (!line) {

                        if (multi_ack == 2 && got_common
                            && !got_other && ok_to_give_up()) {
                                sent_ready = 1;
                        if (multi_ack == 2 && got_common
                            && !got_other && ok_to_give_up()) {
                                sent_ready = 1;


@@ -567,8 +566,7 @@ static void check_non_tip(void)
 static void receive_needs(void)
 {
        struct object_array shallows = OBJECT_ARRAY_INIT;
 static void receive_needs(void)
 {
        struct object_array shallows = OBJECT_ARRAY_INIT;

-       static char line[1000];
-       int len, depth = 0;
+       int depth = 0;

        int has_non_tip = 0;
 
        shallow_nr = 0;
        int has_non_tip = 0;
 
        shallow_nr = 0;


@@ -576,9 +574,9 @@ static void receive_needs(void)
                struct object *o;
                const char *features;
                unsigned char sha1_buf[20];
                struct object *o;
                const char *features;
                unsigned char sha1_buf[20];

-               len = packet_read_line(0, line, sizeof(line));
+               char *line = packet_read_line(0, NULL);

                reset_timeout();
                reset_timeout();

-               if (!len)
+               if (!line)

                        break;
 
                if (!prefixcmp(line, "shallow ")) {
                        break;
 
                if (!prefixcmp(line, "shallow ")) {