Like v1, this is on origin/ms/packet-err-check.
Thanks, Junio, for your reviews.
Jonathan Tan (4):
pkt-line: introduce struct packet_writer
sideband: reverse its dependency on pkt-line
{fetch,upload}-pack: sideband v2 fetch response
tests: define GIT_TEST_SIDEBAND_ALL
Documentation/technical/protocol-v2.txt | 10 ++
fetch-pack.c | 13 +-
pkt-line.c | 121 +++++++++++++++---
pkt-line.h | 34 +++++
sideband.c | 161 ++++++++++++------------
sideband.h | 14 ++-
t/README | 5 +
t/lib-httpd/apache.conf | 1 +
t/t5537-fetch-shallow.sh | 3 +-
t/t5701-git-serve.sh | 2 +-
t/t5702-protocol-v2.sh | 4 +-
upload-pack.c | 131 +++++++++++--------
12 files changed, 343 insertions(+), 156 deletions(-)
Interdiff against v1:
diff --git a/pkt-line.c b/pkt-line.c
index 71b17e6b93..69162f3990 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -447,16 +447,16 @@ int recv_sideband(const char *me, int in_stream, int out)
while (1) {
len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 0);
- retval = diagnose_sideband(me, buf, len, 0);
+ retval = demultiplex_sideband(me, buf, len, 0);
switch (retval) {
- case SIDEBAND_PRIMARY:
- write_or_die(out, buf + 1, len - 1);
- break;
- case SIDEBAND_PROGRESS:
- /* already written by diagnose_sideband() */
- break;
- default: /* flush or error */
- return retval;
+ case SIDEBAND_PRIMARY:
+ write_or_die(out, buf + 1, len - 1);
+ break;
+ case SIDEBAND_PROGRESS:
+ /* already written by demultiplex_sideband() */
+ break;
+ default: /* errors: message already written */
+ return retval;
}
}
}
@@ -474,6 +474,7 @@ void packet_reader_init(struct packet_reader *reader, int fd,
reader->buffer = packet_buffer;
reader->buffer_size = sizeof(packet_buffer);
reader->options = options;
+ reader->me = "git";
}
enum packet_read_status packet_reader_read(struct packet_reader *reader)
@@ -483,6 +484,10 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
return reader->status;
}
+ /*
+ * Consume all progress and keepalive packets until a primary payload
+ * packet is received
+ */
while (1) {
int retval;
reader->status = packet_read_with_status(reader->fd,
@@ -494,29 +499,31 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
reader->options);
if (!reader->use_sideband)
break;
- retval = diagnose_sideband(reader->me, reader->buffer,
- reader->pktlen, 1);
+ retval = demultiplex_sideband(reader->me, reader->buffer,
+ reader->pktlen, 1);
switch (retval) {
- case SIDEBAND_PROTOCOL_ERROR:
- case SIDEBAND_REMOTE_ERROR:
- BUG("should have died in diagnose_sideband");
- case SIDEBAND_FLUSH:
- goto nonprogress;
- case SIDEBAND_PRIMARY:
- if (reader->pktlen != 1)
- goto nonprogress;
- /*
- * Since pktlen is 1, this is a keepalive
- * packet. Wait for the next one.
- */
- break;
- default: /* SIDEBAND_PROGRESS */
- ;
+ case SIDEBAND_PROTOCOL_ERROR:
+ case SIDEBAND_REMOTE_ERROR:
+ BUG("should have died in diagnose_sideband");
+ case SIDEBAND_FLUSH:
+ goto nonprogress_received;
+ case SIDEBAND_PRIMARY:
+ if (reader->pktlen != 1)
+ goto nonprogress_received;
+ /*
+ * Since the packet contains nothing but the sideband
+ * designator, this is a keepalive packet. Wait for the
+ * next one.
+ */
+ break;
+ default: /* SIDEBAND_PROGRESS */
+ ;
}
}
-nonprogress:
+nonprogress_received:
if (reader->status == PACKET_READ_NORMAL)
+ /* Skip the sideband designator if sideband is used */
reader->line = reader->use_sideband ?
reader->buffer + 1 : reader->buffer;
else
@@ -549,7 +556,7 @@ void packet_writer_write(struct packet_writer *writer, const char *fmt, ...)
va_start(args, fmt);
packet_write_fmt_1(writer->dest_fd, 0,
- writer->use_sideband ? "\1" : "", fmt, args);
+ writer->use_sideband ? "\001" : "", fmt, args);
va_end(args);
}
@@ -559,7 +566,7 @@ void packet_writer_error(struct packet_writer *writer, const char *fmt, ...)
va_start(args, fmt);
packet_write_fmt_1(writer->dest_fd, 0,
- writer->use_sideband ? "\3" : "ERR ", fmt, args);
+ writer->use_sideband ? "\003" : "ERR ", fmt, args);
va_end(args);
}
diff --git a/sideband.c b/sideband.c
index cbeab380ae..9d3051e3fe 100644
--- a/sideband.c
+++ b/sideband.c
@@ -113,7 +113,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
#define ANSI_SUFFIX "\033[K"
#define DUMB_SUFFIX " "
-int diagnose_sideband(const char *me, char *buf, int len, int die_on_error)
+int demultiplex_sideband(const char *me, char *buf, int len, int die_on_error)
{
static const char *suffix;
struct strbuf outbuf = STRBUF_INIT;
diff --git a/sideband.h b/sideband.h
index 3786670694..f75c4fde2a 100644
--- a/sideband.h
+++ b/sideband.h
@@ -8,15 +8,14 @@
#define SIDEBAND_PROGRESS 2
/*
- * buf and len should be the result of reading a line from a remote sending
- * multiplexed data.
+ * Inspects a multiplexed packet read from the remote and returns which
+ * sideband it is for.
*
- * Determines the nature of the result and returns it. If
- * SIDEBAND_PROTOCOL_ERROR, SIDEBAND_REMOTE_ERROR, or SIDEBAND_PROGRESS, also
- * prints a message (or the formatted contents of the notice in the case of
- * SIDEBAND_PROGRESS) to stderr.
+ * If SIDEBAND_PROTOCOL_ERROR, SIDEBAND_REMOTE_ERROR, or SIDEBAND_PROGRESS,
+ * also prints a message (or the formatted contents of the notice in the case
+ * of SIDEBAND_PROGRESS) to stderr.
*/
-int diagnose_sideband(const char *me, char *buf, int len, int die_on_error);
+int demultiplex_sideband(const char *me, char *buf, int len, int die_on_error);
void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
--
2.19.0.271.gfe8321ec05.dirty
