Hi Adam, comments/questions below (mostly simple stuff, nothing major):
* IntegerPolynomial.java
o The comment block for multByInt should reflect your changes you
made, namely the removal of "r" from the signature.
o 88: In the case of multiplying two very large long values would
we ever see those cause an overflow. If so, is it OK to have
them overflow before the reduce operation?
o 420-425: Looks like this was a copy of the comment block from
conditionalSwap(). Maybe needs to be tailored to what the
conditionalAssign method does?
o IntegerPolynomialP256.java, IntegerPolynomialP384.java,
IntegerPolynomialP521.java
+ In the carryReduce0 and carryReduce methods, you have many
uses of an integer literal (33554432 for P256, 134217728 for
P384/521). Should these be made as private static final
long values?
o P256OrderField.java, P384OrderField.java, P521OrderField.java
+ Similar question about using a static final long vs.
repeated int literal values in the carryReduce* methods.
o With respect to the last two main bullet items: Would conversion
to a static final value be difficult due to the fact that they
are generated from Fieldgen.jsh?
--Jamil
On 11/30/2018 11:59 AM, Adam Petcher wrote:
JBS: https://bugs.openjdk.java.net/browse/JDK-8208648
webrev: http://cr.openjdk.java.net/~apetcher/8208648/webrev.00/
This changeset includes the enhancements to the finite field
arithmetic library that are necessary for the new implementation of
ECDSA and ECDH[1]. The actual ECDH and ECDSA changes will be reviewed
separately. Please see the JBS tickets for more information about the
changes, and here are some additional notes:
1) This change includes a code generator that produces finite field
implementations. The generated code is included in the review, and it
will be checked in to the repository with a comment indicating that it
is generated and should not be modified directly. Another option is to
put the code generator into the build process so the generated code
does not need to be checked in, but it's not clear whether this is a
better option.
2) Reviewing every line of the generated code is probably not
necessary, and it is probably better to focus on the code generator
itself (Fieldgen.jsh). Of course, it is probably a good idea to review
the generated code for its structure and for opportunities for
improvement.
[1] https://bugs.openjdk.java.net/browse/JDK-8208698
