code review 14438068: go.crypto/ssh: (un)marshal data without type byte prefix.

go.crypto/ssh: (un)marshal data without type byte prefix.

This helps manipulating data in global and channel request
payloads.

[hanwen-google, 2013-10-14 20:46:11]

Hello agl1, dfc, jpsugar@google.com (cc: golang-dev@googlegroups.com),

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

[jpsugar, 2013-10-14 20:48:33]

LGTM

[agl1, 2013-10-14 20:52:20]

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go
File ssh/messages.go (right):

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode231
ssh/messages.go:231: // reflection.  expectedType, if positive, is the expected
SSH message
expectedType is unsigned, so always positive. Do you mean "if non-zero"?

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode231
ssh/messages.go:231: // reflection.  expectedType, if positive, is the expected
SSH message
single space after period.

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode326
ssh/messages.go:326: // prepended if it is positive.
s/positive/non-zero.

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode329
ssh/messages.go:329: if msgType > 0 {
out := make([]byte, 0, 64)
if msgType > 0 {
  out = append(out, msgType)
}

[hanwen-google, 2013-10-14 21:16:24]

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go
File ssh/messages.go (right):

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode231
ssh/messages.go:231: // reflection.  expectedType, if positive, is the expected
SSH message
On 2013/10/14 20:52:20, agl1 wrote:
> single space after period.

Done.

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode231
ssh/messages.go:231: // reflection.  expectedType, if positive, is the expected
SSH message
On 2013/10/14 20:52:20, agl1 wrote:
> expectedType is unsigned, so always positive. Do you mean "if non-zero"?

positive numbers are non-zero by definition? Agree that non-zero is clearer,
though.

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode326
ssh/messages.go:326: // prepended if it is positive.
On 2013/10/14 20:52:20, agl1 wrote:
> s/positive/non-zero.

Done.

https://codereview.appspot.com/14438068/diff/7001/ssh/messages.go#newcode329
ssh/messages.go:329: if msgType > 0 {
On 2013/10/14 20:52:20, agl1 wrote:
> out := make([]byte, 0, 64)
> if msgType > 0 {
>   out = append(out, msgType)
> }

Done.

[dfc, 2013-10-15 04:00:00]

LGTM.

https://codereview.appspot.com/14438068/diff/11001/ssh/messages.go
File ssh/messages.go (right):

https://codereview.appspot.com/14438068/diff/11001/ssh/messages.go#newcode232
ssh/messages.go:232: // type that the packet should start with.  It either
returns nil on
s/It/unmarshal

[hanwen-google, 2013-10-15 05:14:58]

https://codereview.appspot.com/14438068/diff/11001/ssh/messages.go
File ssh/messages.go (right):

https://codereview.appspot.com/14438068/diff/11001/ssh/messages.go#newcode232
ssh/messages.go:232: // type that the packet should start with.  It either
returns nil on
On 2013/10/15 04:00:01, dfc wrote:
> s/It/unmarshal

Done.

[dfc, 2013-10-15 05:17:35]

Thanks, submitting.

On Tue, Oct 15, 2013 at 4:14 PM,  <hanwen@google.com> wrote:
>
> https://codereview.appspot.com/14438068/diff/11001/ssh/messages.go
> File ssh/messages.go (right):
>
> https://codereview.appspot.com/14438068/diff/11001/ssh/messages.go#newcode232
> ssh/messages.go:232: // type that the packet should start with.  It
> either returns nil on
> On 2013/10/15 04:00:01, dfc wrote:
>>
>> s/It/unmarshal
>
>
> Done.
>
>
> https://codereview.appspot.com/14438068/
>
> --
>
> ---You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
>
https://groups.google.com/d/msgid/golang-dev/047d7b15afd580fe0804e8c0aaed%40g....
>
> For more options, visit https://groups.google.com/groups/opt_out.

[dfc, 2013-10-15 05:20:06]

*** Submitted as
https://code.google.com/p/go/source/detail?r=ef3d7138b0b5&repo=crypto ***

go.crypto/ssh: (un)marshal data without type byte prefix.

This helps manipulating data in global and channel request
payloads.

R=agl, dave, jpsugar
CC=golang-dev
https://codereview.appspot.com/14438068

Committer: Dave Cheney <dave@cheney.net>

[hanwen-google, 2013-10-15 14:42:58]

*** Abandoned ***