Re: [PATCH 11/13] remote-curl: accept empty line as terminator

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 30, 2010, 01:39 UTC
Message-ID: <20100830013928.GE2305@burratino>
In-Reply-To: <1283053540-27042-12-git-send-email-srabbelier@gmail.com>

Sverre Rabbelier wrote:

>   I noticed this when my tests suddenly broke. Bisecting pointed at
>   the 'more rigorous return value checking' patch

Shouldn't this go before "check status code of finish_command" for bisectability, then?

>   I'm not very sure about the error message, if anyone feels it
>   should go (it indicates a bug in the remote helper infrastructure,
>   not a user error) it can be left out as far as I'm concerned.

No preference here.

Show 8 quoted lines

> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -813,6 +813,8 @@ int main(int argc, const char **argv)
>  	do {
>  		if (strbuf_getline(&buf, stdin, '\n') == EOF)
>  			break;
> +		if (buf.len == 0)
> +			break;

This is just a bug, I think. Other strbuf_getline() invocations in that file all use the equivalent

	if (*buf->buf)
		break;

too.

Show 7 quoted lines

> @@ -851,6 +853,7 @@ int main(int argc, const char **argv)
>  			printf("\n");
>  			fflush(stdout);
>  		} else {
> +			fprintf(stderr, "Unknown command '%s'\n", buf.buf);
>  			return 1;
>  		}

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>