sideband: reverse its dependency on pkt-line

A subsequent patch will teach struct packet_reader a new field that, if set, instructs it to interpret read data as multiplexed. This will create a dependency from pkt-line to sideband. To avoid a circular dependency, split recv_sideband() into 2 parts: the reading loop (left in recv_sideband()) and the processing of the contents (in demultiplex_sideband()), and move the former into pkt-line. This reverses the direction of dependency: sideband no longer depends on pkt-line, and pkt-line now depends on sideband. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jan 16, 2019 at 11:28 UTC fbd76cd450e6675cbd5d48da3c53fa446b776475
4 files changed +141 -95
pkt-line.c
+23
@@ -439,6 +439,29 @@ ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out)
439 return sb_out->len - orig_len;
440 }
441
442 +int recv_sideband(const char *me, int in_stream, int out)
443 +{
444 + char buf[LARGE_PACKET_MAX + 1];
445 + int len;
446 + struct strbuf scratch = STRBUF_INIT;
447 + enum sideband_type sideband_type;
448 +
449 + while (1) {
450 + len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX,
451 + 0);
452 + if (!demultiplex_sideband(me, buf, len, &scratch,
453 + &sideband_type))
454 + continue;
455 + switch (sideband_type) {
456 + case SIDEBAND_PRIMARY:
457 + write_or_die(out, buf + 1, len - 1);
458 + break;
459 + default: /* errors: message already written */
460 + return sideband_type;
461 + }
462 + }
463 +}
464 +
465 /* Packet Reader Functions */
466 void packet_reader_init(struct packet_reader *reader, int fd,
467 char *src_buffer, size_t src_len,
pkt-line.h
+16
@@ -3,6 +3,7 @@
3
4 #include "git-compat-util.h"
5 #include "strbuf.h"
6 +#include "sideband.h"
7
8 /*
9 * Write a packetized stream, where each line is preceded by
@@ -120,6 +121,21 @@ char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);
121 */
122 ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out);
123
124 +/*
125 + * Receive multiplexed output stream over git native protocol.
126 + * in_stream is the input stream from the remote, which carries data
127 + * in pkt_line format with band designator. Demultiplex it into out
128 + * and err and return error appropriately. Band #1 carries the
129 + * primary payload. Things coming over band #2 is not necessarily
130 + * error; they are usually informative message on the standard error
131 + * stream, aka "verbose"). A message over band #3 is a signal that
132 + * the remote died unexpectedly. A flush() concludes the stream.
133 + *
134 + * Returns SIDEBAND_FLUSH upon a normal conclusion, and SIDEBAND_PROTOCOL_ERROR
135 + * or SIDEBAND_REMOTE_ERROR if an error occurred.
136 + */
137 +int recv_sideband(const char *me, int in_stream, int out);
138 +
139 struct packet_reader {
140 /* source file descriptor */
141 int fd;
sideband.c
+81 -92
@@ -1,7 +1,6 @@
1 #include "cache.h"
2 #include "color.h"
3 #include "config.h"
4 -#include "pkt-line.h"
4 #include "sideband.h"
5 #include "help.h"
6
@@ -109,109 +108,99 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
108 }
109
110
112 -/*
113 - * Receive multiplexed output stream over git native protocol.
114 - * in_stream is the input stream from the remote, which carries data
115 - * in pkt_line format with band designator. Demultiplex it into out
116 - * and err and return error appropriately. Band #1 carries the
117 - * primary payload. Things coming over band #2 is not necessarily
118 - * error; they are usually informative message on the standard error
119 - * stream, aka "verbose"). A message over band #3 is a signal that
120 - * the remote died unexpectedly. A flush() concludes the stream.
121 - */
122 -
111 #define DISPLAY_PREFIX "remote: "
112
113 #define ANSI_SUFFIX "\033[K"
114 #define DUMB_SUFFIX " "
115
128 -int recv_sideband(const char *me, int in_stream, int out)
116 +int demultiplex_sideband(const char *me, char *buf, int len,
117 + struct strbuf *scratch,
118 + enum sideband_type *sideband_type)
119 {
130 - const char *suffix;
131 - char buf[LARGE_PACKET_MAX + 1];
132 - struct strbuf outbuf = STRBUF_INIT;
133 - int retval = 0;
134 -
135 - if (isatty(2) && !is_terminal_dumb())
136 - suffix = ANSI_SUFFIX;
137 - else
138 - suffix = DUMB_SUFFIX;
139 -
140 - while (!retval) {
141 - const char *b, *brk;
142 - int band, len;
143 - len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 0);
144 - if (len == 0)
145 - break;
146 - if (len < 1) {
147 - strbuf_addf(&outbuf,
148 - "%s%s: protocol error: no band designator",
149 - outbuf.len ? "\n" : "", me);
150 - retval = SIDEBAND_PROTOCOL_ERROR;
151 - break;
152 - }
153 - band = buf[0] & 0xff;
154 - buf[len] = '\0';
155 - len--;
156 - switch (band) {
157 - case 3:
158 - strbuf_addf(&outbuf, "%s%s", outbuf.len ? "\n" : "",
159 - DISPLAY_PREFIX);
160 - maybe_colorize_sideband(&outbuf, buf + 1, len);
161 -
162 - retval = SIDEBAND_REMOTE_ERROR;
163 - break;
164 - case 2:
165 - b = buf + 1;
166 -
167 - /*
168 - * Append a suffix to each nonempty line to clear the
169 - * end of the screen line.
170 - *
171 - * The output is accumulated in a buffer and
172 - * each line is printed to stderr using
173 - * write(2) to ensure inter-process atomicity.
174 - */
175 - while ((brk = strpbrk(b, "\n\r"))) {
176 - int linelen = brk - b;
177 -
178 - if (!outbuf.len)
179 - strbuf_addstr(&outbuf, DISPLAY_PREFIX);
180 - if (linelen > 0) {
181 - maybe_colorize_sideband(&outbuf, b, linelen);
182 - strbuf_addstr(&outbuf, suffix);
183 - }
184 -
185 - strbuf_addch(&outbuf, *brk);
186 - xwrite(2, outbuf.buf, outbuf.len);
187 - strbuf_reset(&outbuf);
188 -
189 - b = brk + 1;
190 - }
120 + static const char *suffix;
121 + const char *b, *brk;
122 + int band;
123 +
124 + if (!suffix) {
125 + if (isatty(2) && !is_terminal_dumb())
126 + suffix = ANSI_SUFFIX;
127 + else
128 + suffix = DUMB_SUFFIX;
129 + }
130
192 - if (*b) {
193 - strbuf_addstr(&outbuf, outbuf.len ?
194 - "" : DISPLAY_PREFIX);
195 - maybe_colorize_sideband(&outbuf, b, strlen(b));
131 + if (len == 0) {
132 + *sideband_type = SIDEBAND_FLUSH;
133 + goto cleanup;
134 + }
135 + if (len < 1) {
136 + strbuf_addf(scratch,
137 + "%s%s: protocol error: no band designator",
138 + scratch->len ? "\n" : "", me);
139 + *sideband_type = SIDEBAND_PROTOCOL_ERROR;
140 + goto cleanup;
141 + }
142 + band = buf[0] & 0xff;
143 + buf[len] = '\0';
144 + len--;
145 + switch (band) {
146 + case 3:
147 + strbuf_addf(scratch, "%s%s", scratch->len ? "\n" : "",
148 + DISPLAY_PREFIX);
149 + maybe_colorize_sideband(scratch, buf + 1, len);
150 +
151 + *sideband_type = SIDEBAND_REMOTE_ERROR;
152 + break;
153 + case 2:
154 + b = buf + 1;
155 +
156 + /*
157 + * Append a suffix to each nonempty line to clear the
158 + * end of the screen line.
159 + *
160 + * The output is accumulated in a buffer and
161 + * each line is printed to stderr using
162 + * write(2) to ensure inter-process atomicity.
163 + */
164 + while ((brk = strpbrk(b, "\n\r"))) {
165 + int linelen = brk - b;
166 +
167 + if (!scratch->len)
168 + strbuf_addstr(scratch, DISPLAY_PREFIX);
169 + if (linelen > 0) {
170 + maybe_colorize_sideband(scratch, b, linelen);
171 + strbuf_addstr(scratch, suffix);
172 }
197 - break;
198 - case 1:
199 - write_or_die(out, buf + 1, len);
200 - break;
201 - default:
202 - strbuf_addf(&outbuf, "%s%s: protocol error: bad band #%d",
203 - outbuf.len ? "\n" : "", me, band);
204 - retval = SIDEBAND_PROTOCOL_ERROR;
205 - break;
173 +
174 + strbuf_addch(scratch, *brk);
175 + xwrite(2, scratch->buf, scratch->len);
176 + strbuf_reset(scratch);
177 +
178 + b = brk + 1;
179 + }
180 +
181 + if (*b) {
182 + strbuf_addstr(scratch, scratch->len ?
183 + "" : DISPLAY_PREFIX);
184 + maybe_colorize_sideband(scratch, b, strlen(b));
185 }
186 + return 0;
187 + case 1:
188 + *sideband_type = SIDEBAND_PRIMARY;
189 + break;
190 + default:
191 + strbuf_addf(scratch, "%s%s: protocol error: bad band #%d",
192 + scratch->len ? "\n" : "", me, band);
193 + *sideband_type = SIDEBAND_PROTOCOL_ERROR;
194 + break;
195 }
196
209 - if (outbuf.len) {
210 - strbuf_addch(&outbuf, '\n');
211 - xwrite(2, outbuf.buf, outbuf.len);
197 +cleanup:
198 + if (scratch->len) {
199 + strbuf_addch(scratch, '\n');
200 + xwrite(2, scratch->buf, scratch->len);
201 }
213 - strbuf_release(&outbuf);
214 - return retval;
202 + strbuf_release(scratch);
203 + return 1;
204 }
205
206 /*
sideband.h
+21 -3
@@ -1,10 +1,28 @@
1 #ifndef SIDEBAND_H
2 #define SIDEBAND_H
3
4 -#define SIDEBAND_PROTOCOL_ERROR -2
5 -#define SIDEBAND_REMOTE_ERROR -1
4 +enum sideband_type {
5 + SIDEBAND_PROTOCOL_ERROR = -2,
6 + SIDEBAND_REMOTE_ERROR = -1,
7 + SIDEBAND_FLUSH = 0,
8 + SIDEBAND_PRIMARY = 1
9 +};
10 +
11 +/*
12 + * Inspects a multiplexed packet read from the remote. If this packet is a
13 + * progress packet and thus should not be processed by the caller, returns 0.
14 + * Otherwise, returns 1, releases scratch, and sets sideband_type.
15 + *
16 + * If this packet is SIDEBAND_PROTOCOL_ERROR, SIDEBAND_REMOTE_ERROR, or a
17 + * progress packet, also prints a message to stderr.
18 + *
19 + * scratch must be a struct strbuf allocated by the caller. It is used to store
20 + * progress messages split across multiple packets.
21 + */
22 +int demultiplex_sideband(const char *me, char *buf, int len,
23 + struct strbuf *scratch,
24 + enum sideband_type *sideband_type);
25
7 -int recv_sideband(const char *me, int in_stream, int out);
26 void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
27
28 #endif