Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:50
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] remote-curl: Fix push status report when all branches fail · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH] remote-curl: Fix push status report when all branches fail · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] remote-curl: Fix push status report when all branches fail · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH] remote-curl: Fix push status report when all branches fail · Shawn Pearce <hidden>
- 2016-06-15 · Re: [PATCH] remote-curl: Fix push status report when all branches fail · Jeff King <hidden>
"Shawn O. Pearce" [off-list ref] writes:
Always print a blank line after the send-pack process terminates, ensuring the helper status report (if it was output) will be correctly parsed by the calling transport-helper.c. This ensures the helper doesn't abort before the status report can be shown to the user. Signed-off-by: Shawn O. Pearce <redacted> ---
Anybody wants to add a simple test for this failure mode?
quoted hunk
remote-curl.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-)diff --git a/remote-curl.c b/remote-curl.c index 48c20b8..d6054e2 100644 --- a/remote-curl.c +++ b/remote-curl.c@@ -805,7 +805,7 @@ static int push(int nr_spec, char **specs) static void parse_push(struct strbuf *buf) { char **specs = NULL; - int alloc_spec = 0, nr_spec = 0, i; + int alloc_spec = 0, nr_spec = 0, i, ret; do { if (!prefixcmp(buf->buf, "push ")) {@@ -822,12 +822,11 @@ static void parse_push(struct strbuf *buf) break; } while (1); - if (push(nr_spec, specs)) + ret = push(nr_spec, specs); + xwrite(1, "\n", 1); + if (ret) exit(128); /* error already reported */ - printf("\n"); - fflush(stdout); -
This is not a fault of this patch, but could we fix this ugly mixture of
xwrite() and printf() in the same program? I can see that the loop in the
main() function carefully tries to call fflush(stdout) to make sure that
nothing is pending after processing a single command so using xwrite() may
not cause any harm here, but the thing is that you do not check the error
return from this xwrite(), so use of it is not giving us any potential
benefit of being able to detect I/O errors in a finer grained manner,
i.e. it is no better than the printf("\n"); fflush(stdout); sequence it
replaces.
Thanks.