Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

From: Jeff King <peff@peff.net>
Date: Jul 8, 2016, 22:46 UTC
Message-ID: <20160708224601.GA11389@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kbj8tGb9BsTo_CGqOdfsinJrafHAY5HhQEQDCtAXQqKgw@mail.gmail.com>

On Fri, Jul 08, 2016 at 03:43:27PM -0700, Stefan Beller wrote:

Show 11 quoted lines

> On Fri, Jul 8, 2016 at 3:35 PM, Jeff King <peff@peff.net> wrote:
> >
> > Yes. I haven't been following the intermediate discussion and patches,
> > but I don't see anything wrong with the general design above. I think
> > you do need to use rp_error() to get the die message to the client for
> > non-ssh cases, though (that is a problem with other protocol-error
> > messages, too; I wonder if we should install a custom die handler, or
> > convert them all to some kind of rp_die()).
> 
> Some of the rp_error messages do not want to die(), but most seem to
> be ok when the rp_error would die.

Sorry, I meant converting die() into:

  rp_error(...);
  die(...);

possibly via an rp_die() helper. The existing rp_error() cases would remain untouched.

Installing a die handler could make that work automatically, though I suspect it would lead to corner cases where we break protocol (e.g., if we die() in the middle of writing out a packet).

-Peff