Use packet_reader instead of packet_read_line

By using and sharing a packet_reader while handling a Git pack protocol request, the same reader option is used throughout the code. This makes it easy to set a reader option to the request parsing code. Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Masaya Suzuki committed Dec 29, 2018 at 13:19 UTC 01f9ec64c8a82a05ba7e5a17b292ede037a469ea
6 files changed +129 -108
builtin/archive.c
+10 -9
@@ -27,10 +27,10 @@ static int run_remote_archiver(int argc, const char **argv,
27 const char *remote, const char *exec,
28 const char *name_hint)
29 {
30 - char *buf;
30 int fd[2], i, rv;
31 struct transport *transport;
32 struct remote *_remote;
33 + struct packet_reader reader;
34
35 _remote = remote_get(remote);
36 if (!_remote->url[0])
@@ -53,18 +53,19 @@ static int run_remote_archiver(int argc, const char **argv,
53 packet_write_fmt(fd[1], "argument %s\n", argv[i]);
54 packet_flush(fd[1]);
55
56 - buf = packet_read_line(fd[0], NULL);
57 - if (!buf)
56 + packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE);
57 +
58 + if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
59 die(_("git archive: expected ACK/NAK, got a flush packet"));
59 - if (strcmp(buf, "ACK")) {
60 - if (starts_with(buf, "NACK "))
61 - die(_("git archive: NACK %s"), buf + 5);
62 - if (starts_with(buf, "ERR "))
63 - die(_("remote error: %s"), buf + 4);
60 + if (strcmp(reader.line, "ACK")) {
61 + if (starts_with(reader.line, "NACK "))
62 + die(_("git archive: NACK %s"), reader.line + 5);
63 + if (starts_with(reader.line, "ERR "))
64 + die(_("remote error: %s"), reader.line + 4);
65 die(_("git archive: protocol error"));
66 }
67
67 - if (packet_read_line(fd[0], NULL))
68 + if (packet_reader_read(&reader) != PACKET_READ_FLUSH)
69 die(_("git archive: expected a flush"));
70
71 /* Now, start reading from fd[0] and spit it out to stdout */
builtin/receive-pack.c
+31 -29
@@ -1569,30 +1569,29 @@ static void queue_commands_from_cert(struct command **tail,
1569 }
1570 }
1571
1572 -static struct command *read_head_info(struct oid_array *shallow)
1572 +static struct command *read_head_info(struct packet_reader *reader,
1573 + struct oid_array *shallow)
1574 {
1575 struct command *commands = NULL;
1576 struct command **p = &commands;
1577 for (;;) {
1577 - char *line;
1578 - int len, linelen;
1578 + int linelen;
1579
1580 - line = packet_read_line(0, &len);
1581 - if (!line)
1580 + if (packet_reader_read(reader) != PACKET_READ_NORMAL)
1581 break;
1582
1584 - if (len > 8 && starts_with(line, "shallow ")) {
1583 + if (reader->pktlen > 8 && starts_with(reader->line, "shallow ")) {
1584 struct object_id oid;
1586 - if (get_oid_hex(line + 8, &oid))
1585 + if (get_oid_hex(reader->line + 8, &oid))
1586 die("protocol error: expected shallow sha, got '%s'",
1588 - line + 8);
1587 + reader->line + 8);
1588 oid_array_append(shallow, &oid);
1589 continue;
1590 }
1591
1593 - linelen = strlen(line);
1594 - if (linelen < len) {
1595 - const char *feature_list = line + linelen + 1;
1592 + linelen = strlen(reader->line);
1593 + if (linelen < reader->pktlen) {
1594 + const char *feature_list = reader->line + linelen + 1;
1595 if (parse_feature_request(feature_list, "report-status"))
1596 report_status = 1;
1597 if (parse_feature_request(feature_list, "side-band-64k"))
@@ -1607,28 +1606,32 @@ static struct command *read_head_info(struct oid_array *shallow)
1606 use_push_options = 1;
1607 }
1608
1610 - if (!strcmp(line, "push-cert")) {
1609 + if (!strcmp(reader->line, "push-cert")) {
1610 int true_flush = 0;
1612 - char certbuf[1024];
1611 + int saved_options = reader->options;
1612 + reader->options &= ~PACKET_READ_CHOMP_NEWLINE;
1613
1614 for (;;) {
1615 - len = packet_read(0, NULL, NULL,
1616 - certbuf, sizeof(certbuf), 0);
1617 - if (!len) {
1615 + packet_reader_read(reader);
1616 + if (reader->status == PACKET_READ_FLUSH) {
1617 true_flush = 1;
1618 break;
1619 }
1621 - if (!strcmp(certbuf, "push-cert-end\n"))
1620 + if (reader->status != PACKET_READ_NORMAL) {
1621 + die("protocol error: got an unexpected packet");
1622 + }
1623 + if (!strcmp(reader->line, "push-cert-end\n"))
1624 break; /* end of cert */
1623 - strbuf_addstr(&push_cert, certbuf);
1625 + strbuf_addstr(&push_cert, reader->line);
1626 }
1627 + reader->options = saved_options;
1628
1629 if (true_flush)
1630 break;
1631 continue;
1632 }
1633
1631 - p = queue_command(p, line, linelen);
1634 + p = queue_command(p, reader->line, linelen);
1635 }
1636
1637 if (push_cert.len)
@@ -1637,18 +1640,14 @@ static struct command *read_head_info(struct oid_array *shallow)
1640 return commands;
1641 }
1642
1640 -static void read_push_options(struct string_list *options)
1643 +static void read_push_options(struct packet_reader *reader,
1644 + struct string_list *options)
1645 {
1646 while (1) {
1643 - char *line;
1644 - int len;
1645 -
1646 - line = packet_read_line(0, &len);
1647 -
1648 - if (!line)
1647 + if (packet_reader_read(reader) != PACKET_READ_NORMAL)
1648 break;
1649
1651 - string_list_append(options, line);
1650 + string_list_append(options, reader->line);
1651 }
1652 }
1653
@@ -1924,6 +1923,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
1923 struct oid_array shallow = OID_ARRAY_INIT;
1924 struct oid_array ref = OID_ARRAY_INIT;
1925 struct shallow_info si;
1926 + struct packet_reader reader;
1927
1928 struct option options[] = {
1929 OPT__QUIET(&quiet, N_("quiet")),
@@ -1986,12 +1986,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
1986 if (advertise_refs)
1987 return 0;
1988
1989 - if ((commands = read_head_info(&shallow)) != NULL) {
1989 + packet_reader_init(&reader, 0, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
1990 +
1991 + if ((commands = read_head_info(&reader, &shallow)) != NULL) {
1992 const char *unpack_status = NULL;
1993 struct string_list push_options = STRING_LIST_INIT_DUP;
1994
1995 if (use_push_options)
1994 - read_push_options(&push_options);
1996 + read_push_options(&reader, &push_options);
1997 if (!check_cert_push_options(&push_options)) {
1998 struct command *cmd;
1999 for (cmd = commands; cmd; cmd = cmd->next)
fetch-pack.c
+34 -27
@@ -135,38 +135,42 @@ enum ack_type {
135 ACK_ready
136 };
137
138 -static void consume_shallow_list(struct fetch_pack_args *args, int fd)
138 +static void consume_shallow_list(struct fetch_pack_args *args,
139 + struct packet_reader *reader)
140 {
141 if (args->stateless_rpc && args->deepen) {
142 /* If we sent a depth we will get back "duplicate"
143 * shallow and unshallow commands every time there
144 * is a block of have lines exchanged.
145 */
145 - char *line;
146 - while ((line = packet_read_line(fd, NULL))) {
147 - if (starts_with(line, "shallow "))
146 + while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
147 + if (starts_with(reader->line, "shallow "))
148 continue;
149 - if (starts_with(line, "unshallow "))
149 + if (starts_with(reader->line, "unshallow "))
150 continue;
151 die(_("git fetch-pack: expected shallow list"));
152 }
153 + if (reader->status != PACKET_READ_FLUSH)
154 + die(_("git fetch-pack: expected a flush packet after shallow list"));
155 }
156 }
157
156 -static enum ack_type get_ack(int fd, struct object_id *result_oid)
158 +static enum ack_type get_ack(struct packet_reader *reader,
159 + struct object_id *result_oid)
160 {
161 int len;
159 - char *line = packet_read_line(fd, &len);
162 const char *arg;
163
162 - if (!line)
164 + if (packet_reader_read(reader) != PACKET_READ_NORMAL)
165 die(_("git fetch-pack: expected ACK/NAK, got a flush packet"));
164 - if (!strcmp(line, "NAK"))
166 + len = reader->pktlen;
167 +
168 + if (!strcmp(reader->line, "NAK"))
169 return NAK;
166 - if (skip_prefix(line, "ACK ", &arg)) {
170 + if (skip_prefix(reader->line, "ACK ", &arg)) {
171 if (!get_oid_hex(arg, result_oid)) {
172 arg += 40;
169 - len -= arg - line;
173 + len -= arg - reader->line;
174 if (len < 1)
175 return ACK;
176 if (strstr(arg, "continue"))
@@ -178,9 +182,9 @@ static enum ack_type get_ack(int fd, struct object_id *result_oid)
182 return ACK;
183 }
184 }
181 - if (skip_prefix(line, "ERR ", &arg))
185 + if (skip_prefix(reader->line, "ERR ", &arg))
186 die(_("remote error: %s"), arg);
183 - die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
187 + die(_("git fetch-pack: expected ACK/NAK, got '%s'"), reader->line);
188 }
189
190 static void send_request(struct fetch_pack_args *args,
@@ -248,10 +252,14 @@ static int find_common(struct fetch_negotiator *negotiator,
252 int got_ready = 0;
253 struct strbuf req_buf = STRBUF_INIT;
254 size_t state_len = 0;
255 + struct packet_reader reader;
256
257 if (args->stateless_rpc && multi_ack == 1)
258 die(_("--stateless-rpc requires multi_ack_detailed"));
259
260 + packet_reader_init(&reader, fd[0], NULL, 0,
261 + PACKET_READ_CHOMP_NEWLINE);
262 +
263 if (!args->no_dependents) {
264 mark_tips(negotiator, args->negotiation_tips);
265 for_each_cached_alternate(negotiator, insert_one_alternate_object);
@@ -336,31 +344,30 @@ static int find_common(struct fetch_negotiator *negotiator,
344 state_len = req_buf.len;
345
346 if (args->deepen) {
339 - char *line;
347 const char *arg;
348 struct object_id oid;
349
350 send_request(args, fd[1], &req_buf);
344 - while ((line = packet_read_line(fd[0], NULL))) {
345 - if (skip_prefix(line, "shallow ", &arg)) {
351 + while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
352 + if (skip_prefix(reader.line, "shallow ", &arg)) {
353 if (get_oid_hex(arg, &oid))
347 - die(_("invalid shallow line: %s"), line);
354 + die(_("invalid shallow line: %s"), reader.line);
355 register_shallow(the_repository, &oid);
356 continue;
357 }
351 - if (skip_prefix(line, "unshallow ", &arg)) {
358 + if (skip_prefix(reader.line, "unshallow ", &arg)) {
359 if (get_oid_hex(arg, &oid))
353 - die(_("invalid unshallow line: %s"), line);
360 + die(_("invalid unshallow line: %s"), reader.line);
361 if (!lookup_object(the_repository, oid.hash))
355 - die(_("object not found: %s"), line);
362 + die(_("object not found: %s"), reader.line);
363 /* make sure that it is parsed as shallow */
364 if (!parse_object(the_repository, &oid))
358 - die(_("error in object: %s"), line);
365 + die(_("error in object: %s"), reader.line);
366 if (unregister_shallow(&oid))
360 - die(_("no shallow found: %s"), line);
367 + die(_("no shallow found: %s"), reader.line);
368 continue;
369 }
363 - die(_("expected shallow/unshallow, got %s"), line);
370 + die(_("expected shallow/unshallow, got %s"), reader.line);
371 }
372 } else if (!args->stateless_rpc)
373 send_request(args, fd[1], &req_buf);
@@ -397,9 +404,9 @@ static int find_common(struct fetch_negotiator *negotiator,
404 if (!args->stateless_rpc && count == INITIAL_FLUSH)
405 continue;
406
400 - consume_shallow_list(args, fd[0]);
407 + consume_shallow_list(args, &reader);
408 do {
402 - ack = get_ack(fd[0], result_oid);
409 + ack = get_ack(&reader, result_oid);
410 if (ack)
411 print_verbose(args, _("got %s %d %s"), "ack",
412 ack, oid_to_hex(result_oid));
@@ -469,9 +476,9 @@ done:
476 strbuf_release(&req_buf);
477
478 if (!got_ready || !no_done)
472 - consume_shallow_list(args, fd[0]);
479 + consume_shallow_list(args, &reader);
480 while (flushes || multi_ack) {
474 - int ack = get_ack(fd[0], result_oid);
481 + int ack = get_ack(&reader, result_oid);
482 if (ack) {
483 print_verbose(args, _("got %s (%d) %s"), "ack",
484 ack, oid_to_hex(result_oid));
remote-curl.c
+15 -7
@@ -409,28 +409,36 @@ static struct discovery *discover_refs(const char *service, int for_push)
409 if (maybe_smart &&
410 (5 <= last->len && last->buf[4] == '#') &&
411 !strbuf_cmp(&exp, &type)) {
412 - char *line;
412 + struct packet_reader reader;
413 + packet_reader_init(&reader, -1, last->buf, last->len,
414 + PACKET_READ_CHOMP_NEWLINE);
415
416 /*
417 * smart HTTP response; validate that the service
418 * pkt-line matches our request.
419 */
418 - line = packet_read_line_buf(&last->buf, &last->len, NULL);
419 - if (!line)
420 + if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
421 die("invalid server response; expected service, got flush packet");
422
423 strbuf_reset(&exp);
424 strbuf_addf(&exp, "# service=%s", service);
424 - if (strcmp(line, exp.buf))
425 - die("invalid server response; got '%s'", line);
425 + if (strcmp(reader.line, exp.buf))
426 + die("invalid server response; got '%s'", reader.line);
427 strbuf_release(&exp);
428
429 /* The header can include additional metadata lines, up
430 * until a packet flush marker. Ignore these now, but
431 * in the future we might start to scan them.
432 */
432 - while (packet_read_line_buf(&last->buf, &last->len, NULL))
433 - ;
433 + for (;;) {
434 + packet_reader_read(&reader);
435 + if (reader.pktlen <= 0) {
436 + break;
437 + }
438 + }
439 +
440 + last->buf = reader.src_buffer;
441 + last->len = reader.src_len;
442
443 last->proto_git = 1;
444 } else if (maybe_smart &&
send-pack.c
+19 -18
@@ -135,38 +135,36 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *extra, struc
135 return 0;
136 }
137
138 -static int receive_unpack_status(int in)
138 +static int receive_unpack_status(struct packet_reader *reader)
139 {
140 - const char *line = packet_read_line(in, NULL);
141 - if (!line)
140 + if (packet_reader_read(reader) != PACKET_READ_NORMAL)
141 return error(_("unexpected flush packet while reading remote unpack status"));
143 - if (!skip_prefix(line, "unpack ", &line))
144 - return error(_("unable to parse remote unpack status: %s"), line);
145 - if (strcmp(line, "ok"))
146 - return error(_("remote unpack failed: %s"), line);
142 + if (!skip_prefix(reader->line, "unpack ", &reader->line))
143 + return error(_("unable to parse remote unpack status: %s"), reader->line);
144 + if (strcmp(reader->line, "ok"))
145 + return error(_("remote unpack failed: %s"), reader->line);
146 return 0;
147 }
148
150 -static int receive_status(int in, struct ref *refs)
149 +static int receive_status(struct packet_reader *reader, struct ref *refs)
150 {
151 struct ref *hint;
152 int ret;
153
154 hint = NULL;
156 - ret = receive_unpack_status(in);
155 + ret = receive_unpack_status(reader);
156 while (1) {
158 - char *refname;
157 + const char *refname;
158 char *msg;
160 - char *line = packet_read_line(in, NULL);
161 - if (!line)
159 + if (packet_reader_read(reader) != PACKET_READ_NORMAL)
160 break;
163 - if (!starts_with(line, "ok ") && !starts_with(line, "ng ")) {
164 - error("invalid ref status from remote: %s", line);
161 + if (!starts_with(reader->line, "ok ") && !starts_with(reader->line, "ng ")) {
162 + error("invalid ref status from remote: %s", reader->line);
163 ret = -1;
164 break;
165 }
166
169 - refname = line + 3;
167 + refname = reader->line + 3;
168 msg = strchr(refname, ' ');
169 if (msg)
170 *msg++ = '\0';
@@ -187,7 +185,7 @@ static int receive_status(int in, struct ref *refs)
185 continue;
186 }
187
190 - if (line[0] == 'o' && line[1] == 'k')
188 + if (reader->line[0] == 'o' && reader->line[1] == 'k')
189 hint->status = REF_STATUS_OK;
190 else {
191 hint->status = REF_STATUS_REMOTE_REJECT;
@@ -390,6 +388,7 @@ int send_pack(struct send_pack_args *args,
388 int ret;
389 struct async demux;
390 const char *push_cert_nonce = NULL;
391 + struct packet_reader reader;
392
393 /* Does the other end support the reporting? */
394 if (server_supports("report-status"))
@@ -559,6 +558,8 @@ int send_pack(struct send_pack_args *args,
558 in = demux.out;
559 }
560
561 + packet_reader_init(&reader, in, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
562 +
563 if (need_pack_data && cmds_sent) {
564 if (pack_objects(out, remote_refs, extra_have, args) < 0) {
565 for (ref = remote_refs; ref; ref = ref->next)
@@ -573,7 +574,7 @@ int send_pack(struct send_pack_args *args,
574 * are failing, and just want the error() side effects.
575 */
576 if (status_report)
576 - receive_unpack_status(in);
577 + receive_unpack_status(&reader);
578
579 if (use_sideband) {
580 close(demux.out);
@@ -590,7 +591,7 @@ int send_pack(struct send_pack_args *args,
591 packet_flush(out);
592
593 if (status_report && cmds_sent)
593 - ret = receive_status(in, remote_refs);
594 + ret = receive_status(&reader, remote_refs);
595 else
596 ret = 0;
597 if (args->stateless_rpc)
upload-pack.c
+20 -18
@@ -354,7 +354,8 @@ static int ok_to_give_up(const struct object_array *have_obj,
354 min_generation);
355 }
356
357 -static int get_common_commits(struct object_array *have_obj,
357 +static int get_common_commits(struct packet_reader *reader,
358 + struct object_array *have_obj,
359 struct object_array *want_obj)
360 {
361 struct object_id oid;
@@ -366,12 +367,11 @@ static int get_common_commits(struct object_array *have_obj,
367 save_commit_buffer = 0;
368
369 for (;;) {
369 - char *line = packet_read_line(0, NULL);
370 const char *arg;
371
372 reset_timeout();
373
374 - if (!line) {
374 + if (packet_reader_read(reader) != PACKET_READ_NORMAL) {
375 if (multi_ack == 2 && got_common
376 && !got_other && ok_to_give_up(have_obj, want_obj)) {
377 sent_ready = 1;
@@ -390,7 +390,7 @@ static int get_common_commits(struct object_array *have_obj,
390 got_other = 0;
391 continue;
392 }
393 - if (skip_prefix(line, "have ", &arg)) {
393 + if (skip_prefix(reader->line, "have ", &arg)) {
394 switch (got_oid(arg, &oid, have_obj)) {
395 case -1: /* they have what we do not */
396 got_other = 1;
@@ -416,7 +416,7 @@ static int get_common_commits(struct object_array *have_obj,
416 }
417 continue;
418 }
419 - if (!strcmp(line, "done")) {
419 + if (!strcmp(reader->line, "done")) {
420 if (have_obj->nr > 0) {
421 if (multi_ack)
422 packet_write_fmt(1, "ACK %s\n", last_hex);
@@ -425,7 +425,7 @@ static int get_common_commits(struct object_array *have_obj,
425 packet_write_fmt(1, "NAK\n");
426 return -1;
427 }
428 - die("git upload-pack: expected SHA1 list, got '%s'", line);
428 + die("git upload-pack: expected SHA1 list, got '%s'", reader->line);
429 }
430 }
431
@@ -826,7 +826,7 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,
826 return 0;
827 }
828
829 -static void receive_needs(struct object_array *want_obj)
829 +static void receive_needs(struct packet_reader *reader, struct object_array *want_obj)
830 {
831 struct object_array shallows = OBJECT_ARRAY_INIT;
832 struct string_list deepen_not = STRING_LIST_INIT_DUP;
@@ -840,33 +840,32 @@ static void receive_needs(struct object_array *want_obj)
840 struct object *o;
841 const char *features;
842 struct object_id oid_buf;
843 - char *line = packet_read_line(0, NULL);
843 const char *arg;
844
845 reset_timeout();
847 - if (!line)
846 + if (packet_reader_read(reader) != PACKET_READ_NORMAL)
847 break;
848
850 - if (process_shallow(line, &shallows))
849 + if (process_shallow(reader->line, &shallows))
850 continue;
852 - if (process_deepen(line, &depth))
851 + if (process_deepen(reader->line, &depth))
852 continue;
854 - if (process_deepen_since(line, &deepen_since, &deepen_rev_list))
853 + if (process_deepen_since(reader->line, &deepen_since, &deepen_rev_list))
854 continue;
856 - if (process_deepen_not(line, &deepen_not, &deepen_rev_list))
855 + if (process_deepen_not(reader->line, &deepen_not, &deepen_rev_list))
856 continue;
857
859 - if (skip_prefix(line, "filter ", &arg)) {
858 + if (skip_prefix(reader->line, "filter ", &arg)) {
859 if (!filter_capability_requested)
860 die("git upload-pack: filtering capability not negotiated");
861 parse_list_objects_filter(&filter_options, arg);
862 continue;
863 }
864
866 - if (!skip_prefix(line, "want ", &arg) ||
865 + if (!skip_prefix(reader->line, "want ", &arg) ||
866 parse_oid_hex(arg, &oid_buf, &features))
867 die("git upload-pack: protocol error, "
869 - "expected to get object ID, not '%s'", line);
868 + "expected to get object ID, not '%s'", reader->line);
869
870 if (parse_feature_request(features, "deepen-relative"))
871 deepen_relative = 1;
@@ -1055,6 +1054,7 @@ void upload_pack(struct upload_pack_options *options)
1054 {
1055 struct string_list symref = STRING_LIST_INIT_DUP;
1056 struct object_array want_obj = OBJECT_ARRAY_INIT;
1057 + struct packet_reader reader;
1058
1059 stateless_rpc = options->stateless_rpc;
1060 timeout = options->timeout;
@@ -1078,10 +1078,12 @@ void upload_pack(struct upload_pack_options *options)
1078 if (options->advertise_refs)
1079 return;
1080
1081 - receive_needs(&want_obj);
1081 + packet_reader_init(&reader, 0, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
1082 +
1083 + receive_needs(&reader, &want_obj);
1084 if (want_obj.nr) {
1085 struct object_array have_obj = OBJECT_ARRAY_INIT;
1084 - get_common_commits(&have_obj, &want_obj);
1086 + get_common_commits(&reader, &have_obj, &want_obj);
1087 create_pack_file(&have_obj, &want_obj);
1088 }
1089 }