Re: [PATCH] remote-curl: Fix push status report when all branches fail

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Shawn O. Pearce" <spearce@xxxxxxxxxxx> 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 <spearce@xxxxxxxxxxx>
> ---

Anybody wants to add a simple test for this failure mode?

>  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]