{"thread":{"id":"55500","subject":"[PATCH] pkt-line: do not report packet write errors twice","startedAt":"2021-04-15T19:10:24Z","lastAt":"2021-04-15T21:59:35Z","messageCount":8,"participants":["Matheus Tavares","Junio C Hamano","Matheus Tavares Bernardino"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"422049","messageId":"64b81865fdfcc505b6aa3e6ebaebf3b9ccb36eb1.1618513583.git.matheus.bernardino@usp.br","threadId":"55500","inReplyTo":null,"subject":"[PATCH] pkt-line: do not report packet write errors twice","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-15T19:10:07Z","receivedAt":"2021-04-15T19:10:24Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On write() errors, packet_write() dies with the same error message that\nis already printed by its callee, packet_write_gently(). This produces\nan unnecessarily verbose and repetitive output:\n\nerror: packet write failed\nfatal: packet write failed: \n\nIn addition to that, packet_write_gently() does not always fulfill its\ncaller expectation that errno will be properly set before a non-zero\nreturn. In particular, that is not the case for a \"data exceeds max\npacket size\" error. So, in this case, packet_write() will call\ndie_errno() and print a strerror(errno) message that might be totally\nunrelated to the actual error.\n\nFix both those issues by turning packet_write() and\npacket_write_gently() into wrappers to a lower level function which is\nnow responsible to either error() or die() as requested by its caller.\n\nSigned-off-by: Matheus Tavares \n---\n pkt-line.c | 27 ++++++++++++++++++++-------\n 1 file changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 0194137528..39c9ca4212 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -194,13 +194,18 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n \treturn status;\n }\n \n-static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n+static int do_packet_write(const int fd_out, const char *buf, size_t size,\n+\t\t\t int gentle)\n {\n \tchar header[4];\n \tsize_t packet_size;\n \n-\tif (size > LARGE_PACKET_DATA_MAX)\n-\t\treturn error(_(\"packet write failed - data exceeds max packet size\"));\n+\tif (size > LARGE_PACKET_DATA_MAX) {\n+\t\tif (gentle)\n+\t\t\treturn error(_(\"packet write failed - data exceeds max packet size\"));\n+\t\telse\n+\t\t\tdie(_(\"packet write failed - data exceeds max packet size\"));\n+\t}\n \n \tpacket_trace(buf, size, 1);\n \tpacket_size = size + 4;\n@@ -215,15 +220,23 @@ static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n \t */\n \n \tif (write_in_full(fd_out, header, 4) < 0 ||\n-\t write_in_full(fd_out, buf, size) < 0)\n-\t\treturn error(_(\"packet write failed\"));\n+\t write_in_full(fd_out, buf, size) < 0) {\n+\t\tif (gentle)\n+\t\t\treturn error_errno(_(\"packet write failed\"));\n+\t\telse\n+\t\t\tdie_errno(_(\"packet write failed\"));\n+\t}\n \treturn 0;\n }\n \n+static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n+{\n+\treturn do_packet_write(fd_out, buf, size, 1);\n+}\n+\n void packet_write(int fd_out, const char *buf, size_t size)\n {\n-\tif (packet_write_gently(fd_out, buf, size))\n-\t\tdie_errno(_(\"packet write failed\"));\n+\tdo_packet_write(fd_out, buf, size, 0);\n }\n \n void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n-- \n2.30.1\n\n"},{"id":"422052","messageId":"xmqqtuo7pb2b.fsf@gitster.g","threadId":"55500","inReplyTo":"64b81865fdfcc505b6aa3e6ebaebf3b9ccb36eb1.1618513583.git.matheus.bernardino@usp.br","subject":"Re: [PATCH] pkt-line: do not report packet write errors twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-15T20:06:20Z","receivedAt":"2021-04-15T20:06:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares writes:\n\n> On write() errors, packet_write() dies with the same error message that\n> is already printed by its callee, packet_write_gently(). This produces\n> an unnecessarily verbose and repetitive output:\n>\n> error: packet write failed\n> fatal: packet write failed: \n>\n> In addition to that, packet_write_gently() does not always fulfill its\n> caller expectation that errno will be properly set before a non-zero\n> return. In particular, that is not the case for a \"data exceeds max\n> packet size\" error. So, in this case, packet_write() will call\n> die_errno() and print a strerror(errno) message that might be totally\n> unrelated to the actual error.\n>\n> Fix both those issues by turning packet_write() and\n> packet_write_gently() into wrappers to a lower level function which is\n> now responsible to either error() or die() as requested by its caller.\n>\n> Signed-off-by: Matheus Tavares \n> ---\n> pkt-line.c | 27 ++++++++++++++++++++-------\n> 1 file changed, 20 insertions(+), 7 deletions(-)\n\nNicely done. Duplicated error message literals do look, eh,\nrepetitious, though.\n\nI wonder if something like this on top would be an improvement.\n\nUpon seeing \"return error(_(VARIABLE_NAME))\", it may be distracting\nthat you now have to move to where the actual message is defined\nwhile following the logic of the code, but as long as the variable\nname captures the essense of what the message says, it may be OK.\n\nI dunno.\n\n\n pkt-line.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git c/pkt-line.c w/pkt-line.c\nindex 39c9ca4212..d357c74fcd 100644\n--- c/pkt-line.c\n+++ w/pkt-line.c\n@@ -199,12 +199,16 @@ static int do_packet_write(const int fd_out, cons\n {\n \tchar header[4];\n \tsize_t packet_size;\n+\tstatic const char size_error_message[] =\n+\t\tN_(\"packet write failed - data exceeds max packet size\");\n+\tstatic const char write_error_message[] =\n+\t\tN_(\"packet write failed\");\n \n \tif (size > LARGE_PACKET_DATA_MAX) {\n \t\tif (gentle)\n-\t\t\treturn error(_(\"packet write failed - data exceeds max packet size\"));\n+\t\t\treturn error(_(size_error_message));\n \t\telse\n-\t\t\tdie(_(\"packet write failed - data exceeds max packet size\"));\n+\t\t\tdie(_(size_error_message));\n \t}\n \n \tpacket_trace(buf, size, 1);\n@@ -222,9 +226,9 @@ static int do_packet_write(const int fd_out, const\n \tif (write_in_full(fd_out, header, 4) < 0 ||\n \t write_in_full(fd_out, buf, size) < 0) {\n \t\tif (gentle)\n-\t\t\treturn error_errno(_(\"packet write failed\"));\n+\t\t\treturn error_errno(_(write_error_message));\n \t\telse\n-\t\t\tdie_errno(_(\"packet write failed\"));\n+\t\t\tdie_errno(_(write_error_message));\n \t}\n \treturn 0;\n }\n"},{"id":"422054","messageId":"CAHd-oW4sqJpeJywgRJCkchX0uZFSxD-UnguRJcRcQQywd+scbA@mail.gmail.com","threadId":"55500","inReplyTo":"xmqqtuo7pb2b.fsf@gitster.g","subject":"Re: [PATCH] pkt-line: do not report packet write errors twice","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-15T20:24:52Z","receivedAt":"2021-04-15T20:25:09Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Thu, Apr 15, 2021 at 5:06 PM Junio C Hamano wrote:\n>\n> Matheus Tavares writes:\n>\n> > On write() errors, packet_write() dies with the same error message that\n> > is already printed by its callee, packet_write_gently(). This produces\n> > an unnecessarily verbose and repetitive output:\n> >\n> > error: packet write failed\n> > fatal: packet write failed: \n> >\n> > In addition to that, packet_write_gently() does not always fulfill its\n> > caller expectation that errno will be properly set before a non-zero\n> > return. In particular, that is not the case for a \"data exceeds max\n> > packet size\" error. So, in this case, packet_write() will call\n> > die_errno() and print a strerror(errno) message that might be totally\n> > unrelated to the actual error.\n> >\n> > Fix both those issues by turning packet_write() and\n> > packet_write_gently() into wrappers to a lower level function which is\n> > now responsible to either error() or die() as requested by its caller.\n> >\n> > Signed-off-by: Matheus Tavares \n> > ---\n> > pkt-line.c | 27 ++++++++++++++++++++-------\n> > 1 file changed, 20 insertions(+), 7 deletions(-)\n>\n> Nicely done. Duplicated error message literals do look, eh,\n> repetitious, though.\n>\n> I wonder if something like this on top would be an improvement.\n>\n> Upon seeing \"return error(_(VARIABLE_NAME))\", it may be distracting\n> that you now have to move to where the actual message is defined\n> while following the logic of the code, but as long as the variable\n> name captures the essense of what the message says, it may be OK.\n>\n> I dunno.\n>\n>\n> pkt-line.c | 12 ++++++++----\n> 1 file changed, 8 insertions(+), 4 deletions(-)\n>\n> diff --git c/pkt-line.c w/pkt-line.c\n> index 39c9ca4212..d357c74fcd 100644\n> --- c/pkt-line.c\n> +++ w/pkt-line.c\n> @@ -199,12 +199,16 @@ static int do_packet_write(const int fd_out, cons\n> {\n> char header[4];\n> size_t packet_size;\n> + static const char size_error_message[] =\n> + N_(\"packet write failed - data exceeds max packet size\");\n> + static const char write_error_message[] =\n> + N_(\"packet write failed\");\n>\n> if (size > LARGE_PACKET_DATA_MAX) {\n> if (gentle)\n> - return error(_(\"packet write failed - data exceeds max packet size\"));\n> + return error(_(size_error_message));\n> else\n> - die(_(\"packet write failed - data exceeds max packet size\"));\n> + die(_(size_error_message));\n> }\n>\n> packet_trace(buf, size, 1);\n> @@ -222,9 +226,9 @@ static int do_packet_write(const int fd_out, const\n> if (write_in_full(fd_out, header, 4) < 0 ||\n> write_in_full(fd_out, buf, size) < 0) {\n> if (gentle)\n> - return error_errno(_(\"packet write failed\"));\n> + return error_errno(_(write_error_message));\n> else\n> - die_errno(_(\"packet write failed\"));\n> + die_errno(_(write_error_message));\n> }\n> return 0;\n> }\n\nNice! :) Maybe we could also avoid the static strings without\nrepeating the literals by making `do_packet_write()` receive a `struct\nstrbuf *err` and save the error message in it? Then the two callers\ncan decide whether to pass it to error() or die() accordingly.\n"},{"id":"422055","messageId":"xmqqlf9jp9v5.fsf@gitster.g","threadId":"55500","inReplyTo":"CAHd-oW4sqJpeJywgRJCkchX0uZFSxD-UnguRJcRcQQywd+scbA@mail.gmail.com","subject":"Re: [PATCH] pkt-line: do not report packet write errors twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-15T20:32:14Z","receivedAt":"2021-04-15T20:32:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares Bernardino writes:\n\n> Nice! :) Maybe we could also avoid the static strings without\n> repeating the literals by making `do_packet_write()` receive a `struct\n> strbuf *err` and save the error message in it? Then the two callers\n> can decide whether to pass it to error() or die() accordingly.\n\nSorry, but I do not understand what benefit we are trying to gain by\nintroducing an extra strbuf (which I assume is used to strbuf_add()\nthe error message into). Wouldn't the caller need two messages and\nflip between vs pair?\n\n\n"},{"id":"422057","messageId":"20210415204809.1818959-1-matheus.bernardino@usp.br","threadId":"55500","inReplyTo":"xmqqlf9jp9v5.fsf@gitster.g","subject":"Re: [PATCH] pkt-line: do not report packet write errors twice","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-15T20:48:09Z","receivedAt":"2021-04-15T20:48:19Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Thu, Apr 15, 2021 at 5:32 PM Junio C Hamano wrote:\n>\n> Matheus Tavares Bernardino writes:\n>\n> > Nice! :) Maybe we could also avoid the static strings without\n> > repeating the literals by making `do_packet_write()` receive a `struct\n> > strbuf *err` and save the error message in it? Then the two callers\n> > can decide whether to pass it to error() or die() accordingly.\n>\n> Sorry, but I do not understand what benefit we are trying to gain by\n> introducing an extra strbuf (which I assume is used to strbuf_add()\n> the error message into).  Wouldn't the caller need two messages and\n> flip between vs pair?\n\nHmm, I'm not sure I understand what you mean by the two messages, but\nwhat I was thinking of is something like this (on top of my original\npatch):\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 39c9ca4212..98304ce374 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -195,16 +195,14 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n }\n\n static int do_packet_write(const int fd_out, const char *buf, size_t size,\n-\t\t\t int gentle)\n+\t\t\t struct strbuf *err)\n {\n \tchar header[4];\n \tsize_t packet_size;\n\n \tif (size > LARGE_PACKET_DATA_MAX) {\n-\t\tif (gentle)\n-\t\t\treturn error(_(\"packet write failed - data exceeds max packet size\"));\n-\t\telse\n-\t\t\tdie(_(\"packet write failed - data exceeds max packet size\"));\n+\t\tstrbuf_addstr(err, _(\"packet write failed - data exceeds max packet size\"));\n+\t\treturn -1;\n \t}\n\n \tpacket_trace(buf, size, 1);\n@@ -221,22 +219,28 @@ static int do_packet_write(const int fd_out, const char *buf, size_t size,\n\n \tif (write_in_full(fd_out, header, 4) < 0 ||\n \t write_in_full(fd_out, buf, size) < 0) {\n-\t\tif (gentle)\n-\t\t\treturn error_errno(_(\"packet write failed\"));\n-\t\telse\n-\t\t\tdie_errno(_(\"packet write failed\"));\n+\t\tstrbuf_addf(err, _(\"packet write failed: %s\"), strerror(errno));\n+\t\treturn -1;\n \t}\n \treturn 0;\n }\n\n static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n {\n-\treturn do_packet_write(fd_out, buf, size, 1);\n+\tstruct strbuf err = STRBUF_INIT;\n+\tif (do_packet_write(fd_out, buf, size, &err)) {\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\treturn -1;\n+\t}\n+\treturn 0;\n }\n\n void packet_write(int fd_out, const char *buf, size_t size)\n {\n-\tdo_packet_write(fd_out, buf, size, 0);\n+\tstruct strbuf err = STRBUF_INIT;\n+\tif (do_packet_write(fd_out, buf, size, &err))\n+\t\tdie(\"%s\", err.buf);\n }\n\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n\n"},{"id":"422059","messageId":"xmqqczuvp8ra.fsf@gitster.g","threadId":"55500","inReplyTo":"20210415204809.1818959-1-matheus.bernardino@usp.br","subject":"Re: [PATCH] pkt-line: do not report packet write errors twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-15T20:56:09Z","receivedAt":"2021-04-15T20:56:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares writes:\n\n> On Thu, Apr 15, 2021 at 5:32 PM Junio C Hamano wrote:\n>>\n>> Matheus Tavares Bernardino writes:\n>>\n>> > Nice! :) Maybe we could also avoid the static strings without\n>> > repeating the literals by making `do_packet_write()` receive a `struct\n>> > strbuf *err` and save the error message in it? Then the two callers\n>> > can decide whether to pass it to error() or die() accordingly.\n>>\n>> Sorry, but I do not understand what benefit we are trying to gain by\n>> introducing an extra strbuf (which I assume is used to strbuf_add()\n>> the error message into).  Wouldn't the caller need two messages and\n>> flip between vs pair?\n>\n> Hmm, I'm not sure I understand what you mean by the two messages, but\n> what I was thinking of is something like this (on top of my original\n> patch):\n\nAh, OK, you meant that you'd make do_packet_write() handle\ndie/error_errno itself by manually calling strerror(errno).\n\nOne small downside with the approach is that do_packet_write() needs\nto hardcode how die/error_errno() mixes the strerror() with the\ncaller supplied error message, but I do not care too strongly either\nway. My original motivation of suggesting the rewrite was to avoid\noverlong lines in the main flow of the code by replacing the long\nmessages with variable names that are much shorter, so as long as\nthat is done, I am fine either way.\n\nThanks.\n"},{"id":"422066","messageId":"d477a069eed1cfa10ebe68991caed6c2b0ebb43b.1618522570.git.matheus.bernardino@usp.br","threadId":"55500","inReplyTo":"64b81865fdfcc505b6aa3e6ebaebf3b9ccb36eb1.1618513583.git.matheus.bernardino@usp.br","subject":"[PATCH v2] pkt-line: do not report packet write errors twice","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-15T21:57:52Z","receivedAt":"2021-04-15T21:58:04Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On write() errors, packet_write() dies with the same error message that\nis already printed by its callee, packet_write_gently(). This produces\nan unnecessarily verbose and repetitive output:\n\nerror: packet write failed\nfatal: packet write failed: \n\nIn addition to that, packet_write_gently() does not always fulfill its\ncaller expectation that errno will be properly set before a non-zero\nreturn. In particular, that is not the case for a \"data exceeds max\npacket size\" error. So, in this case, packet_write() will call\ndie_errno() and print an strerror(errno) message that might be totally\nunrelated to the actual error.\n\nFix both those issues by turning packet_write() and\npacket_write_gently() into wrappers to a common lower level function\nthat doesn't print the error message, but instead returns it on a buffer\nfor the caller to die() or error() as appropriate.\n\nSigned-off-by: Matheus Tavares \n---\n pkt-line.c | 31 ++++++++++++++++++++++++-------\n 1 file changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 0194137528..98304ce374 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -194,13 +194,16 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n \treturn status;\n }\n \n-static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n+static int do_packet_write(const int fd_out, const char *buf, size_t size,\n+\t\t\t struct strbuf *err)\n {\n \tchar header[4];\n \tsize_t packet_size;\n \n-\tif (size > LARGE_PACKET_DATA_MAX)\n-\t\treturn error(_(\"packet write failed - data exceeds max packet size\"));\n+\tif (size > LARGE_PACKET_DATA_MAX) {\n+\t\tstrbuf_addstr(err, _(\"packet write failed - data exceeds max packet size\"));\n+\t\treturn -1;\n+\t}\n \n \tpacket_trace(buf, size, 1);\n \tpacket_size = size + 4;\n@@ -215,15 +218,29 @@ static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n \t */\n \n \tif (write_in_full(fd_out, header, 4) < 0 ||\n-\t write_in_full(fd_out, buf, size) < 0)\n-\t\treturn error(_(\"packet write failed\"));\n+\t write_in_full(fd_out, buf, size) < 0) {\n+\t\tstrbuf_addf(err, _(\"packet write failed: %s\"), strerror(errno));\n+\t\treturn -1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tif (do_packet_write(fd_out, buf, size, &err)) {\n+\t\terror(\"%s\", err.buf);\n+\t\tstrbuf_release(&err);\n+\t\treturn -1;\n+\t}\n \treturn 0;\n }\n \n void packet_write(int fd_out, const char *buf, size_t size)\n {\n-\tif (packet_write_gently(fd_out, buf, size))\n-\t\tdie_errno(_(\"packet write failed\"));\n+\tstruct strbuf err = STRBUF_INIT;\n+\tif (do_packet_write(fd_out, buf, size, &err))\n+\t\tdie(\"%s\", err.buf);\n }\n \n void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n-- \n2.30.1\n\n"},{"id":"422067","messageId":"xmqqtuo7nr97.fsf@gitster.g","threadId":"55500","inReplyTo":"d477a069eed1cfa10ebe68991caed6c2b0ebb43b.1618522570.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v2] pkt-line: do not report packet write errors twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-15T21:59:32Z","receivedAt":"2021-04-15T21:59:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares writes:\n\n> On write() errors, packet_write() dies with the same error message that\n> is already printed by its callee, packet_write_gently(). This produces\n> an unnecessarily verbose and repetitive output:\n>\n> error: packet write failed\n> fatal: packet write failed: \n>\n> In addition to that, packet_write_gently() does not always fulfill its\n> caller expectation that errno will be properly set before a non-zero\n> return. In particular, that is not the case for a \"data exceeds max\n> packet size\" error. So, in this case, packet_write() will call\n> die_errno() and print an strerror(errno) message that might be totally\n> unrelated to the actual error.\n>\n> Fix both those issues by turning packet_write() and\n> packet_write_gently() into wrappers to a common lower level function\n> that doesn't print the error message, but instead returns it on a buffer\n> for the caller to die() or error() as appropriate.\n>\n> Signed-off-by: Matheus Tavares \n> ---\n> pkt-line.c | 31 ++++++++++++++++++++++++-------\n> 1 file changed, 24 insertions(+), 7 deletions(-)\n\nNicely done. Thanks, will queue.\n\n>\n> diff --git a/pkt-line.c b/pkt-line.c\n> index 0194137528..98304ce374 100644\n> --- a/pkt-line.c\n> +++ b/pkt-line.c\n> @@ -194,13 +194,16 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n> \treturn status;\n> }\n> \n> -static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n> +static int do_packet_write(const int fd_out, const char *buf, size_t size,\n> +\t\t\t struct strbuf *err)\n> {\n> \tchar header[4];\n> \tsize_t packet_size;\n> \n> -\tif (size > LARGE_PACKET_DATA_MAX)\n> -\t\treturn error(_(\"packet write failed - data exceeds max packet size\"));\n> +\tif (size > LARGE_PACKET_DATA_MAX) {\n> +\t\tstrbuf_addstr(err, _(\"packet write failed - data exceeds max packet size\"));\n> +\t\treturn -1;\n> +\t}\n> \n> \tpacket_trace(buf, size, 1);\n> \tpacket_size = size + 4;\n> @@ -215,15 +218,29 @@ static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n> \t */\n> \n> \tif (write_in_full(fd_out, header, 4) < 0 ||\n> -\t write_in_full(fd_out, buf, size) < 0)\n> -\t\treturn error(_(\"packet write failed\"));\n> +\t write_in_full(fd_out, buf, size) < 0) {\n> +\t\tstrbuf_addf(err, _(\"packet write failed: %s\"), strerror(errno));\n> +\t\treturn -1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n> +{\n> +\tstruct strbuf err = STRBUF_INIT;\n> +\tif (do_packet_write(fd_out, buf, size, &err)) {\n> +\t\terror(\"%s\", err.buf);\n> +\t\tstrbuf_release(&err);\n> +\t\treturn -1;\n> +\t}\n> \treturn 0;\n> }\n> \n> void packet_write(int fd_out, const char *buf, size_t size)\n> {\n> -\tif (packet_write_gently(fd_out, buf, size))\n> -\t\tdie_errno(_(\"packet write failed\"));\n> +\tstruct strbuf err = STRBUF_INIT;\n> +\tif (do_packet_write(fd_out, buf, size, &err))\n> +\t\tdie(\"%s\", err.buf);\n> }\n> \n> void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n"}]}