Subject: Re: [RFC PATCH v3 6/8] Support remote helpers implementing smart transports

From: Ilari Liusvaara <[email-masked]>
Date: Mon, 7 Dec 2009 22:35:24 +0200

On Mon, Dec 07, 2009 at 10:11:48AM -0800, Shawn O. Pearce wrote:
> 
> We should already be connected because of the prior call into
> get_refs_list().  If I read your code correctly we'd try to open
> a new connection right here, which makes no sense. 

The have prior connection case can't happen since take_over_transport()
overwrites the method pointers.

> But its also
> possible for us to be in a different transport, so we do code with
> the assumption that we didn't get invoked through get_refs_list()
> first and therefore need to open the connection ourselves.

Right. The reason why the code is there is in case somebody invokes
fetch() first.

The same things apply to push function too.

> Also, given the above invocation pattern, I see no reason why you
> need the disown virtual function on struct transport*.  Just pass
> the #@!**! struct child* into transport_take_over() from the 3
> call sites here and get rid of that unnecessary indirection.

Fixed.

-Ilari