Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(77)

Issue 8431007: crypto: add simple P224 implementation. (Closed)

Created:
9 years, 1 month ago by agl
Modified:
7 years ago
Reviewers:
Nico, Wez
CC:
chromium-reviews
Visibility:
Public.

Description

crypto: add simple P224 implementation. This is intended to be the underlying group for an EKE implementation for Remoting. BUG=none TEST=crypto_unittests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108866

Patch Set 1 #

Total comments: 63

Patch Set 2 : ... #

Patch Set 3 : ... #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+1518 lines, -0 lines) Patch
M crypto/crypto.gyp View 2 chunks +3 lines, -0 lines 0 comments Download
A crypto/p224.h View 1 2 1 chunk +55 lines, -0 lines 4 comments Download
A crypto/p224.cc View 1 2 1 chunk +652 lines, -0 lines 0 comments Download
A crypto/p224_unittest.cc View 1 2 1 chunk +808 lines, -0 lines 1 comment Download

Messages

Total messages: 13 (0 generated)
agl
9 years, 1 month ago (2011-10-31 16:18:22 UTC) #1
agl
Poke. You can't bug me for code and then ignore the code review :) Cheers ...
9 years, 1 month ago (2011-11-02 14:59:40 UTC) #2
Wez
I've had a stab at highlighting where I think things could be clarified, although I ...
9 years, 1 month ago (2011-11-02 23:46:06 UTC) #3
Lambros
Removing myself from reviewers list. I don't really have any extra math knowledge that would ...
9 years, 1 month ago (2011-11-03 02:13:19 UTC) #4
agl
http://codereview.chromium.org/8431007/diff/1/crypto/p224.cc File crypto/p224.cc (right): http://codereview.chromium.org/8431007/diff/1/crypto/p224.cc#newcode27 crypto/p224.cc:27: typedef crypto::P224::FieldElement FieldElement; On 2011/11/02 23:46:06, Wez wrote: > ...
9 years, 1 month ago (2011-11-03 17:20:49 UTC) #5
Wez
This is much more readable, IMHO. Thanks! Couple more comments, below... http://codereview.chromium.org/8431007/diff/1/crypto/p224.cc File crypto/p224.cc (right): ...
9 years, 1 month ago (2011-11-03 20:03:46 UTC) #6
agl
I still need to do the check that the arguments to Add aren't both the ...
9 years, 1 month ago (2011-11-04 20:13:22 UTC) #7
Wez
LGTM! http://codereview.chromium.org/8431007/diff/15001/crypto/p224.h File crypto/p224.h (right): http://codereview.chromium.org/8431007/diff/15001/crypto/p224.h#newcode28 crypto/p224.h:28: bool Set(const base::StringPiece& in); nit: The external representation ...
9 years, 1 month ago (2011-11-04 22:13:46 UTC) #8
agl
http://codereview.chromium.org/8431007/diff/15001/crypto/p224.h File crypto/p224.h (right): http://codereview.chromium.org/8431007/diff/15001/crypto/p224.h#newcode28 crypto/p224.h:28: bool Set(const base::StringPiece& in); On 2011/11/04 22:13:46, Wez wrote: ...
9 years, 1 month ago (2011-11-07 15:15:12 UTC) #9
Ryan Sleevi
Random drive-by: It was reverted because it broke the shared builders. You need to add ...
9 years, 1 month ago (2011-11-07 16:08:12 UTC) #10
agl
On Mon, Nov 7, 2011 at 11:08 AM, <rsleevi@chromium.org> wrote: > It was reverted because ...
9 years, 1 month ago (2011-11-07 16:24:52 UTC) #11
Nico
https://codereview.chromium.org/8431007/diff/15001/crypto/p224_unittest.cc File crypto/p224_unittest.cc (right): https://codereview.chromium.org/8431007/diff/15001/crypto/p224_unittest.cc#newcode805 crypto/p224_unittest.cc:805: EXPECT_TRUE(memcmp(&sum, &a, sizeof(sum) != 0)); A warning I'm prototyping ...
7 years ago (2013-12-21 05:50:43 UTC) #12
agl
7 years ago (2013-12-21 12:37:51 UTC) #13
On Sat, Dec 21, 2013 at 12:50 AM,  <thakis@chromium.org> wrote:
>
> https://codereview.chromium.org/8431007/diff/15001/crypto/p224_unittest.cc
> File crypto/p224_unittest.cc (right):
>
>
https://codereview.chromium.org/8431007/diff/15001/crypto/p224_unittest.cc#ne...
> crypto/p224_unittest.cc:805: EXPECT_TRUE(memcmp(&sum, &a, sizeof(sum) !=
> 0));
> A warning I'm prototyping (http://llvm.org/PR18297) flags this line. The
> warning is correct, right?

Yes, thanks!

I've just landed the fix because I'll forget if I leave it until after
the vacation:
http://src.chromium.org/viewvc/chrome?revision=242277&view=revision


Cheers

AGL

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698