code review 4156044: crypto/openpgp/packet: four more packet types.

crypto/openpgp/packet: four more packet types.

[bradfitz, 2011-02-09 16:07:38]

LGTM

minor comments only

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/literal.go (right):

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/literal.go:13: type LiteralData struct {
Per the spec, you should add a:

func (l *LiteralData) ForEyesOnly() bool {
  return l.FileName == "_CONSOLE"
}

:-)

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/literal.go:16: CreationTime uint32
I assume unix epoch time?

Spec is vague but what else could it be.  Might want to note that 0 means
undefined, rather than 1970.

Also, CreationTime isn't necessarily a correct name.  The spec says modification
time OR creation time OR zero.  Might just want to call it "Time" and add a big
comment.

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/literal.go:43: l.CreationTime = uint32(buf[0])<<24
| uint32(buf[1])<<16 | uint32(buf[2])<<8 | uint32(buf[3])
encoding/binary here?

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/one_pass_signature.go (right):

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/one_pass_signature.go:35: err =
error.UnsupportedError("one-pass-signature packet version")
include stringified buf[0] in error text?

[agl1, 2011-02-10 12:56:29]

Hello bradfitzgo (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[agl1, 2011-02-10 12:56:35]

*** Submitted as http://code.google.com/p/go/source/detail?r=1ac7cdf2a243 ***

crypto/openpgp/packet: four more packet types.

R=bradfitzgo
CC=golang-dev
http://codereview.appspot.com/4156044

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/literal.go (right):

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/literal.go:13: type LiteralData struct {
On 2011/02/09 16:07:38, bradfitzgo wrote:
> Per the spec, you should add a:
> 
> func (l *LiteralData) ForEyesOnly() bool {
>   return l.FileName == "_CONSOLE"
> }
> 
> :-)

Done.

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/literal.go:16: CreationTime uint32
On 2011/02/09 16:07:38, bradfitzgo wrote:
> I assume unix epoch time?
> 
> Spec is vague but what else could it be.  Might want to note that 0 means
> undefined, rather than 1970.
> 
> Also, CreationTime isn't necessarily a correct name.  The spec says
modification
> time OR creation time OR zero.  Might just want to call it "Time" and add a
big
> comment.

Done.

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/literal.go:43: l.CreationTime = uint32(buf[0])<<24
| uint32(buf[1])<<16 | uint32(buf[2])<<8 | uint32(buf[3])
On 2011/02/09 16:07:38, bradfitzgo wrote:
> encoding/binary here?

Done.

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/one_pass_signature.go (right):

http://codereview.appspot.com/4156044/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/one_pass_signature.go:35: err =
error.UnsupportedError("one-pass-signature packet version")
On 2011/02/09 16:07:38, bradfitzgo wrote:
> include stringified buf[0] in error text?

Done.