code review 21940043: gosshnew/ssh: implement client using modular connections.

gosshnew/ssh: use modular connection code on the client side.

[hanwen-google, 2013-11-05 16:13:00]

Hello agl@golang.org, dave@cheney.net (cc: agl@golang.org, dfc@golang.org,
golang-dev@googlegroups.com, hanwen@google.com),

I'd like you to review this change to
https://code.google.com/p/gosshnew

[agl1, 2013-11-05 23:25:20]

LGTM

https://codereview.appspot.com/21940043/diff/40001/ssh/session.go
File ssh/session.go (right):

https://codereview.appspot.com/21940043/diff/40001/ssh/session.go#newcode192
ssh/session.go:192: ok, err := s.ch.SendRequest("pty-req", true, marshal(0,
req))
maybe consider:

const dummyMessageType = 0

just to make the meaning of this 0 clearer.

https://codereview.appspot.com/21940043/diff/40001/ssh/session.go#newcode326
ssh/session.go:326: return fmt.Errorf("ssh: cound not execute")
typo: cound

maybe "ssh: failed to execute shell" or "ssh: could not execute shell"

https://codereview.appspot.com/21940043/diff/40001/ssh/session_test.go
File ssh/session_test.go (left):

https://codereview.appspot.com/21940043/diff/40001/ssh/session_test.go#oldcod...
ssh/session_test.go:383: session.clientChan.sendWindowAdj(0)
I assume that zero sided updates still work? :)

https://codereview.appspot.com/21940043/diff/40001/ssh/tcpip.go
File ssh/tcpip.go (right):

https://codereview.appspot.com/21940043/diff/40001/ssh/tcpip.go#newcode97
ssh/tcpip.go:97: ok, resp, err := c.mux.SendRequest("tcpip-forward", true,
marshal(0, m))
ditto about the zero.

[hanwen-google, 2013-11-06 09:56:38]

https://codereview.appspot.com/21940043/diff/40001/ssh/session.go
File ssh/session.go (right):

https://codereview.appspot.com/21940043/diff/40001/ssh/session.go#newcode192
ssh/session.go:192: ok, err := s.ch.SendRequest("pty-req", true, marshal(0,
req))
On 2013/11/05 23:25:20, agl1 wrote:
> maybe consider:
> 
> const dummyMessageType = 0
> 
> just to make the meaning of this 0 clearer.

I've added a function marshalBare instead.

https://codereview.appspot.com/21940043/diff/40001/ssh/session.go#newcode326
ssh/session.go:326: return fmt.Errorf("ssh: cound not execute")
On 2013/11/05 23:25:20, agl1 wrote:
> typo: cound
> 
> maybe "ssh: failed to execute shell" or "ssh: could not execute shell"

Done.

https://codereview.appspot.com/21940043/diff/40001/ssh/session_test.go
File ssh/session_test.go (left):

https://codereview.appspot.com/21940043/diff/40001/ssh/session_test.go#oldcod...
ssh/session_test.go:383: session.clientChan.sendWindowAdj(0)
On 2013/11/05 23:25:20, agl1 wrote:
> I assume that zero sided updates still work? :)

there is a test for it in mux_test.go that you said looked good :)

https://codereview.appspot.com/21940043/diff/40001/ssh/tcpip.go
File ssh/tcpip.go (right):

https://codereview.appspot.com/21940043/diff/40001/ssh/tcpip.go#newcode97
ssh/tcpip.go:97: ok, resp, err := c.mux.SendRequest("tcpip-forward", true,
marshal(0, m))
On 2013/11/05 23:25:20, agl1 wrote:
> ditto about the zero.

Done.

[hanwen-google, 2013-11-06 10:13:01]

*** Submitted as https://code.google.com/p/gosshnew/source/detail?r=889d17fec13d
***

gosshnew/ssh: use modular connection code on the client side.

R=agl, dave
CC=agl, dfc, golang-dev, hanwen
https://codereview.appspot.com/21940043