Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(996)

Issue 4551044: code review 4551044: crypto/openpgp: add key generation support. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 10 months ago by agl1
Modified:
12 years, 10 months ago
Reviewers:
bradfitz
CC:
r, bradfitzgoog, golang-dev
Visibility:
Public.

Description

crypto/openpgp: add key generation support. This change adds a function for generating new Entities and inchoate support for reserialising Entities.

Patch Set 1 #

Patch Set 2 : diff -r e5ecc416f2fd https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r e5ecc416f2fd https://go.googlecode.com/hg/ #

Total comments: 20

Patch Set 4 : diff -r b2d0fe8068f0 https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r b2d0fe8068f0 https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r b2d0fe8068f0 https://go.googlecode.com/hg/ #

Total comments: 14

Patch Set 7 : diff -r b2d0fe8068f0 https://go.googlecode.com/hg/ #

Patch Set 8 : diff -r b2d0fe8068f0 https://go.googlecode.com/hg/ #

Patch Set 9 : diff -r 561d9021e7a4 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+415 lines, -37 lines) Patch
M src/pkg/crypto/openpgp/keys.go View 1 2 3 4 5 6 2 chunks +103 lines, -0 lines 0 comments Download
M src/pkg/crypto/openpgp/packet/private_key.go View 1 2 3 2 chunks +84 lines, -0 lines 0 comments Download
M src/pkg/crypto/openpgp/packet/public_key.go View 1 2 3 4 5 6 5 chunks +56 lines, -12 lines 0 comments Download
M src/pkg/crypto/openpgp/packet/signature.go View 1 2 3 4 5 6 1 chunk +29 lines, -11 lines 0 comments Download
M src/pkg/crypto/openpgp/packet/userid.go View 1 2 3 4 5 6 2 chunks +56 lines, -0 lines 0 comments Download
M src/pkg/crypto/openpgp/packet/userid_test.go View 1 2 3 4 5 6 7 1 chunk +45 lines, -0 lines 0 comments Download
M src/pkg/crypto/openpgp/write.go View 1 2 chunks +1 line, -14 lines 0 comments Download
M src/pkg/crypto/openpgp/write_test.go View 1 2 3 2 chunks +41 lines, -0 lines 0 comments Download

Messages

Total messages: 10
agl1
Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
12 years, 10 months ago (2011-05-17 17:07:33 UTC) #1
bradfitz
http://codereview.appspot.com/4551044/diff/4001/src/pkg/crypto/openpgp/keys.go File src/pkg/crypto/openpgp/keys.go (right): http://codereview.appspot.com/4551044/diff/4001/src/pkg/crypto/openpgp/keys.go#newcode307 src/pkg/crypto/openpgp/keys.go:307: func NewEntity(rand io.Reader, currentTimeSecs int64, id string) (*Entity, os.Error) ...
12 years, 10 months ago (2011-05-17 17:57:37 UTC) #2
r
http://codereview.appspot.com/4551044/diff/4001/src/pkg/crypto/openpgp/keys.go File src/pkg/crypto/openpgp/keys.go (right): http://codereview.appspot.com/4551044/diff/4001/src/pkg/crypto/openpgp/keys.go#newcode352 src/pkg/crypto/openpgp/keys.go:352: subkey.Sig.IssuerKeyId = &e.PrimaryKey.KeyId would any of this code look ...
12 years, 10 months ago (2011-05-17 18:24:24 UTC) #3
agl1
Hello bradfitz@golang.org, r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
12 years, 10 months ago (2011-05-19 03:40:30 UTC) #4
agl1
http://codereview.appspot.com/4551044/diff/4001/src/pkg/crypto/openpgp/keys.go File src/pkg/crypto/openpgp/keys.go (right): http://codereview.appspot.com/4551044/diff/4001/src/pkg/crypto/openpgp/keys.go#newcode307 src/pkg/crypto/openpgp/keys.go:307: func NewEntity(rand io.Reader, currentTimeSecs int64, id string) (*Entity, os.Error) ...
12 years, 10 months ago (2011-05-19 03:42:55 UTC) #5
r
LGTM for style http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/packet/public_key.go File src/pkg/crypto/openpgp/packet/public_key.go (right): http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/packet/public_key.go#newcode46 src/pkg/crypto/openpgp/packet/public_key.go:46: pk.IsSubkey = isSubkey again, maybe composite ...
12 years, 10 months ago (2011-05-19 04:16:10 UTC) #6
bradfitzgoog
http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/keys.go File src/pkg/crypto/openpgp/keys.go (right): http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/keys.go#newcode307 src/pkg/crypto/openpgp/keys.go:307: // which may be nil but must not contain ...
12 years, 10 months ago (2011-05-19 05:14:38 UTC) #7
agl1
http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/keys.go File src/pkg/crypto/openpgp/keys.go (right): http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/keys.go#newcode307 src/pkg/crypto/openpgp/keys.go:307: // which may be nil but must not contain ...
12 years, 10 months ago (2011-05-19 18:18:39 UTC) #8
agl1
*** Submitted as http://code.google.com/p/go/source/detail?r=0949bacd4903 *** crypto/openpgp: add key generation support. This change adds a function ...
12 years, 10 months ago (2011-05-20 16:37:20 UTC) #9
bradfitz
12 years, 10 months ago (2011-05-20 17:01:22 UTC) #10
LGTM

Thanks again!

http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/signature.go (right):

http://codereview.appspot.com/4551044/diff/9002/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:448: h, err :=
userIdSignatureHash(id, pub, sig)
On 2011/05/19 18:18:39, agl1 wrote:
> On 2011/05/19 05:14:38, bradfitzgoog wrote:
> > you're shadowing err here, no?
> 
> I don't believe so.

Wow, shows what I know.  Just tested it... you're right.  Good to know.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b