{"thread":{"id":"56711","subject":"[PATCH 0/2] pkt-line.[ch]: dead code removal","startedAt":"2021-10-14T20:15:18Z","lastAt":"2021-10-21T16:30:29Z","messageCount":4,"participants":["Ævar Arnfjörð Bjarmason","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"438782","messageId":"cover-0.2-00000000000-20211014T201317Z-avarab@gmail.com","threadId":"56711","inReplyTo":null,"subject":"[PATCH 0/2] pkt-line.[ch]: dead code removal","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-14T20:15:10Z","receivedAt":"2021-10-14T20:15:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove some dead code in pkt-line.[ch], perhaps someone has an\nobjection to 2/2 as we could keep that function variant around \"just\nin case\", but it's trivial to use the underlying function (or re-add\nthis utility), so shedding the unused code seems better.\n\nÆvar Arnfjörð Bjarmason (2):\n  pkt-line.[ch]: remove unused packet_buf_write_len()\n  pkt-line.[ch]: remove unused packet_read_line_buf()\n\n builtin/checkout--worker.c |  4 ++--\n daemon.c                   |  2 +-\n parallel-checkout.c        |  3 +--\n pkt-line.c                 | 45 ++++++--------------------------------\n pkt-line.h                 | 10 +--------\n remote-curl.c              |  2 +-\n 6 files changed, 13 insertions(+), 53 deletions(-)\n\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"438783","messageId":"patch-1.2-4e90690af51-20211014T201317Z-avarab@gmail.com","threadId":"56711","inReplyTo":"cover-0.2-00000000000-20211014T201317Z-avarab@gmail.com","subject":"[PATCH 1/2] pkt-line.[ch]: remove unused packet_buf_write_len()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-14T20:15:11Z","receivedAt":"2021-10-14T20:15:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This function was added in f1f4d8acf40 (pkt-line: add\npacket_buf_write_len function, 2018-03-15) for use in\n0f1dc53f45d (remote-curl: implement stateless-connect command,\n2018-03-15).\n\nIn a97d00799a1 (remote-curl: use post_rpc() for protocol v2 also,\n2019-02-21) that only user of it went away, let's remove it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n pkt-line.c | 16 ----------------\n pkt-line.h |  1 -\n 2 files changed, 17 deletions(-)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex de4a94b437e..11e1adc872b 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -289,22 +289,6 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n \tva_end(args);\n }\n \n-void packet_buf_write_len(struct strbuf *buf, const char *data, size_t len)\n-{\n-\tsize_t orig_len, n;\n-\n-\torig_len = buf->len;\n-\tstrbuf_addstr(buf, \"0000\");\n-\tstrbuf_add(buf, data, len);\n-\tn = buf->len - orig_len;\n-\n-\tif (n > LARGE_PACKET_MAX)\n-\t\tdie(_(\"protocol error: impossibly long line\"));\n-\n-\tset_packet_header(&buf->buf[orig_len], n);\n-\tpacket_trace(data, len, 1);\n-}\n-\n int write_packetized_from_fd_no_flush(int fd_in, int fd_out)\n {\n \tchar *buf = xmalloc(LARGE_PACKET_DATA_MAX);\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 82b95e4bdd3..beb589a8593 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -29,7 +29,6 @@ void packet_buf_delim(struct strbuf *buf);\n void set_packet_header(char *buf, int size);\n void packet_write(int fd_out, const char *buf, size_t size);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n-void packet_buf_write_len(struct strbuf *buf, const char *data, size_t len);\n int packet_flush_gently(int fd);\n int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n int write_packetized_from_fd_no_flush(int fd_in, int fd_out);\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"438784","messageId":"patch-2.2-ef854200f5a-20211014T201317Z-avarab@gmail.com","threadId":"56711","inReplyTo":"cover-0.2-00000000000-20211014T201317Z-avarab@gmail.com","subject":"[PATCH 2/2] pkt-line.[ch]: remove unused packet_read_line_buf()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-14T20:15:12Z","receivedAt":"2021-10-14T20:15:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This function was added in 4981fe750b1 (pkt-line: share\nbuffer/descriptor reading implementation, 2013-02-23), but in\n01f9ec64c8a (Use packet_reader instead of packet_read_line,\n2018-12-29) the code that was using it was removed.\n\nSince it's being removed we can in turn remove the \"src\" and \"src_len\"\narguments to packet_read(), all the remaining users just passed a\nNULL/NULL pair to it.\n\nThat function is only a thin wrapper for packet_read_with_status()\nwhich still needs those arguments, but for the thin packet_read()\nconvenience wrapper we can do away with it for now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/checkout--worker.c |  4 ++--\n daemon.c                   |  2 +-\n parallel-checkout.c        |  3 +--\n pkt-line.c                 | 29 +++++++----------------------\n pkt-line.h                 |  9 +--------\n remote-curl.c              |  2 +-\n 6 files changed, 13 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c\nindex fb9fd13b73c..ede7dc32a43 100644\n--- a/builtin/checkout--worker.c\n+++ b/builtin/checkout--worker.c\n@@ -82,8 +82,8 @@ static void worker_loop(struct checkout *state)\n \tsize_t i, nr = 0, alloc = 0;\n \n \twhile (1) {\n-\t\tint len = packet_read(0, NULL, NULL, packet_buffer,\n-\t\t\t\t      sizeof(packet_buffer), 0);\n+\t\tint len = packet_read(0, packet_buffer, sizeof(packet_buffer),\n+\t\t\t\t      0);\n \n \t\tif (len < 0)\n \t\t\tBUG(\"packet_read() returned negative value\");\ndiff --git a/daemon.c b/daemon.c\nindex 5c4cbad62d0..3b5f9a41ab2 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -770,7 +770,7 @@ static int execute(void)\n \n \tset_keep_alive(0);\n \talarm(init_timeout ? init_timeout : timeout);\n-\tpktlen = packet_read(0, NULL, NULL, packet_buffer, sizeof(packet_buffer), 0);\n+\tpktlen = packet_read(0, packet_buffer, sizeof(packet_buffer), 0);\n \talarm(0);\n \n \tlen = strlen(line);\ndiff --git a/parallel-checkout.c b/parallel-checkout.c\nindex ddc0ff3c064..ed9c9995207 100644\n--- a/parallel-checkout.c\n+++ b/parallel-checkout.c\n@@ -603,8 +603,7 @@ static void gather_results_from_workers(struct pc_worker *workers,\n \t\t\t\tcontinue;\n \n \t\t\tif (pfd->revents & POLLIN) {\n-\t\t\t\tint len = packet_read(pfd->fd, NULL, NULL,\n-\t\t\t\t\t\t      packet_buffer,\n+\t\t\t\tint len = packet_read(pfd->fd, packet_buffer,\n \t\t\t\t\t\t      sizeof(packet_buffer), 0);\n \n \t\t\t\tif (len < 0) {\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 11e1adc872b..2dc8ac274bd 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -437,38 +437,28 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,\n \treturn PACKET_READ_NORMAL;\n }\n \n-int packet_read(int fd, char **src_buffer, size_t *src_len,\n-\t\tchar *buffer, unsigned size, int options)\n+int packet_read(int fd, char *buffer, unsigned size, int options)\n {\n \tint pktlen = -1;\n \n-\tpacket_read_with_status(fd, src_buffer, src_len, buffer, size,\n-\t\t\t\t&pktlen, options);\n+\tpacket_read_with_status(fd, NULL, NULL, buffer, size, &pktlen,\n+\t\t\t\toptions);\n \n \treturn pktlen;\n }\n \n-static char *packet_read_line_generic(int fd,\n-\t\t\t\t      char **src, size_t *src_len,\n-\t\t\t\t      int *dst_len)\n+char *packet_read_line(int fd, int *dst_len)\n {\n-\tint len = packet_read(fd, src, src_len,\n-\t\t\t      packet_buffer, sizeof(packet_buffer),\n+\tint len = packet_read(fd, packet_buffer, sizeof(packet_buffer),\n \t\t\t      PACKET_READ_CHOMP_NEWLINE);\n \tif (dst_len)\n \t\t*dst_len = len;\n \treturn (len > 0) ? packet_buffer : NULL;\n }\n \n-char *packet_read_line(int fd, int *len_p)\n-{\n-\treturn packet_read_line_generic(fd, NULL, NULL, len_p);\n-}\n-\n int packet_read_line_gently(int fd, int *dst_len, char **dst_line)\n {\n-\tint len = packet_read(fd, NULL, NULL,\n-\t\t\t      packet_buffer, sizeof(packet_buffer),\n+\tint len = packet_read(fd, packet_buffer, sizeof(packet_buffer),\n \t\t\t      PACKET_READ_CHOMP_NEWLINE|PACKET_READ_GENTLE_ON_EOF);\n \tif (dst_len)\n \t\t*dst_len = len;\n@@ -477,11 +467,6 @@ int packet_read_line_gently(int fd, int *dst_len, char **dst_line)\n \treturn len;\n }\n \n-char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)\n-{\n-\treturn packet_read_line_generic(-1, src, src_len, dst_len);\n-}\n-\n ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out, int options)\n {\n \tint packet_len;\n@@ -491,7 +476,7 @@ ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out, int options)\n \n \tfor (;;) {\n \t\tstrbuf_grow(sb_out, LARGE_PACKET_DATA_MAX);\n-\t\tpacket_len = packet_read(fd_in, NULL, NULL,\n+\t\tpacket_len = packet_read(fd_in,\n \t\t\t/* strbuf_grow() above always allocates one extra byte to\n \t\t\t * store a '\\0' at the end of the string. packet_read()\n \t\t\t * writes a '\\0' extra byte at the end, too. Let it know\ndiff --git a/pkt-line.h b/pkt-line.h\nindex beb589a8593..467ae013573 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -87,8 +87,7 @@ void packet_fflush(FILE *f);\n #define PACKET_READ_CHOMP_NEWLINE        (1u<<1)\n #define PACKET_READ_DIE_ON_ERR_PACKET    (1u<<2)\n #define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)\n-int packet_read(int fd, char **src_buffer, size_t *src_len, char\n-\t\t*buffer, unsigned size, int options);\n+int packet_read(int fd, char *buffer, unsigned size, int options);\n \n /*\n  * Convert a four hex digit packet line length header into its numeric\n@@ -137,12 +136,6 @@ char *packet_read_line(int fd, int *size);\n  */\n int packet_read_line_gently(int fd, int *size, char **dst_line);\n \n-/*\n- * Same as packet_read_line, but read from a buf rather than a descriptor;\n- * see packet_read for details on how src_* is used.\n- */\n-char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);\n-\n /*\n  * Reads a stream of variable sized packets until a flush packet is detected.\n  */\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 5975103b96a..d69156312bd 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1088,7 +1088,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads,\n \t\trpc->protocol_header = NULL;\n \n \twhile (!err) {\n-\t\tint n = packet_read(rpc->out, NULL, NULL, rpc->buf, rpc->alloc, 0);\n+\t\tint n = packet_read(rpc->out, rpc->buf, rpc->alloc, 0);\n \t\tif (!n)\n \t\t\tbreak;\n \t\trpc->pos = 0;\n-- \n2.33.1.1338.g20da966911a\n\n"},{"id":"439252","messageId":"YXGVotqAxn859JZf@coredump.intra.peff.net","threadId":"56711","inReplyTo":"cover-0.2-00000000000-20211014T201317Z-avarab@gmail.com","subject":"Re: [PATCH 0/2] pkt-line.[ch]: dead code removal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-21T16:30:26Z","receivedAt":"2021-10-21T16:30:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 14, 2021 at 10:15:10PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Remove some dead code in pkt-line.[ch], perhaps someone has an\n> objection to 2/2 as we could keep that function variant around \"just\n> in case\", but it's trivial to use the underlying function (or re-add\n> this utility), so shedding the unused code seems better.\n\nThese both look good to me.\n\nIt's perhaps a little weird to shed the now-always-NULL src arguments\nfrom packet_read() in the second one, since the underlying function we\nwrap still allows them.  But it does make the callers a little simpler,\nand I think if we added any new callers that I'd much rather see them\nuse the struct-oriented packet_reader interface instead. Which is really\nwhy it we can get rid of this function in the first place; it's old callers\nare all using that interface.\n\nIn the long run, I'd be quite happy if could get rid of all of the\nnon-packet_reader calls entirely, but that's a much bigger topic. This\nis a nice incremental step in that direction.\n\n-Peff\n"}]}