Patchwork [2,of,2] chg: provide early exception to user

login
register
mail settings
Submitter Yuya Nishihara
Date March 12, 2016, 2:01 p.m.
Message ID <ac410dea1b240854b5f0.1457791272@mimosa>
Download mbox | patch
Permalink /patch/13824/
State Accepted
Headers show

Comments

Yuya Nishihara - March 12, 2016, 2:01 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1457788650 -32400
#      Sat Mar 12 22:17:30 2016 +0900
# Node ID ac410dea1b240854b5f0c99dddde66c5b19f6d36
# Parent  44c2880005610595f4a098dfc74672a68b93c147
chg: provide early exception to user

See the previous patch for details. Since the socket will be closed by the
server, handleresponse() will never return:

  Traceback (most recent call last):
    ...
  chg: abort: failed to read channel
Jun Wu - March 12, 2016, 7:43 p.m.
This looks reasonable to me.

I think the client may want to kill the server. The server will not
automatically recover even after the user has fixed their broken extension,
because the mtimehash code does not have a change to execute. But anyway
it's already better then having to use strace to find out what's happening.

I would like to apply both this one and "chgserver: wrap ui without calling
its constructor".

On 03/12/2016 02:01 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1457788650 -32400
> #      Sat Mar 12 22:17:30 2016 +0900
> # Node ID ac410dea1b240854b5f0c99dddde66c5b19f6d36
> # Parent  44c2880005610595f4a098dfc74672a68b93c147
> chg: provide early exception to user
>
> See the previous patch for details. Since the socket will be closed by the
> server, handleresponse() will never return:
>
>    Traceback (most recent call last):
>      ...
>    chg: abort: failed to read channel
>
> diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
> --- a/contrib/chg/hgclient.c
> +++ b/contrib/chg/hgclient.c
> @@ -311,9 +311,16 @@ static void readhello(hgclient_t *hgc)
>   {
>   	readchannel(hgc);
>   	context_t *ctx = &hgc->ctx;
> -	if (ctx->ch != 'o')
> -		abortmsg("unexpected channel of hello message (ch = %c)",
> -			 ctx->ch);
> +	if (ctx->ch != 'o') {
> +		char ch = ctx->ch;
> +		if (ch == 'e') {
> +			/* write early error and will exit */
> +			fwrite(ctx->data, sizeof(ctx->data[0]), ctx->datasize,
> +			       stderr);
> +			handleresponse(hgc);
> +		}
> +		abortmsg("unexpected channel of hello message (ch = %c)", ch);
> +	}
>   	enlargecontext(ctx, ctx->datasize + 1);
>   	ctx->data[ctx->datasize] = '\0';
>   	debugmsg("hello received: %s (size = %zu)", ctx->data, ctx->datasize);
>
Yuya Nishihara - March 13, 2016, 4:32 a.m.
On Sat, 12 Mar 2016 19:43:50 +0000, Jun Wu wrote:
> This looks reasonable to me.
> 
> I think the client may want to kill the server. The server will not
> automatically recover even after the user has fixed their broken extension,
> because the mtimehash code does not have a change to execute.

Yep, but that will require more work since readchannel() aborts if it can't
receive enough data.
Pierre-Yves David - March 14, 2016, 2:02 p.m.
On 03/12/2016 02:01 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1457788650 -32400
> #      Sat Mar 12 22:17:30 2016 +0900
> # Node ID ac410dea1b240854b5f0c99dddde66c5b19f6d36
> # Parent  44c2880005610595f4a098dfc74672a68b93c147
> chg: provide early exception to user

Pushed to the clowncopter. Thanks goes to Jun for the review

Patch

diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
--- a/contrib/chg/hgclient.c
+++ b/contrib/chg/hgclient.c
@@ -311,9 +311,16 @@  static void readhello(hgclient_t *hgc)
 {
 	readchannel(hgc);
 	context_t *ctx = &hgc->ctx;
-	if (ctx->ch != 'o')
-		abortmsg("unexpected channel of hello message (ch = %c)",
-			 ctx->ch);
+	if (ctx->ch != 'o') {
+		char ch = ctx->ch;
+		if (ch == 'e') {
+			/* write early error and will exit */
+			fwrite(ctx->data, sizeof(ctx->data[0]), ctx->datasize,
+			       stderr);
+			handleresponse(hgc);
+		}
+		abortmsg("unexpected channel of hello message (ch = %c)", ch);
+	}
 	enlargecontext(ctx, ctx->datasize + 1);
 	ctx->data[ctx->datasize] = '\0';
 	debugmsg("hello received: %s (size = %zu)", ctx->data, ctx->datasize);