> On 7 Apr 2017, at 12:47, Anthony Scarpino <anthony.scarp...@oracle.com> wrote: > > On 04/07/2017 06:58 AM, Chris Hegarty wrote: >> On 06/04/17 21:39, Anthony Scarpino wrote: >>> >>> I'd like to get a review for this performance change to use the existing >>> CounterMode parallelized intrinsic instead of GCTR's own version. The >>> two classes were nearly identical except for the doFinal() method which >>> doesn't belong in CounterMode.java. >>> >>> I could have been more aggressive with this change, but I'm looking to >>> get this into 9, so I stayed away from completely merging GCTR into >>> CounterMode in case of incompatibilities. All tests security and >>> hotspot tests pass. >>> >>> http://cr.openjdk.java.net/~ascarpino/8177784/webrev/ >> >> This change looks good to me. Trivially, the class-level comment in >> GCTR should be updated ( it refers to removed fields ). Also, >> CounterMode.counter could be protected ( rather than package-private ). >> >> -Chris. > > Thanks Chris, > > I left CounterMode.counter as package-private because the package is what > becomes the SunJCE provider. I don't believe there should be any outside > package classes accessing this code. > > I updated the webrev at with the comment update: > http://cr.openjdk.java.net/~ascarpino/8177784/webrev.01/ >
It’s the same pattern for “iv” with CounterMode and FeedbackCipher, although i
prefer protected as well if never accessed by other classes in the same
package, it makes it easier to reason about the code, since i know more about
what can access it.
Suggestion, take it or leave it: personally i would prefer to see an additional
constructor in CounterMode:
GCTR(SymmetricCipher cipher, byte[] initialCounterBlk) {
super(cipher, checkBlockSize(initialCounterBlk));
}
Where new CounterMode constructor performs the clone, assignmentm and reset,
the static method checkBlockSize checks the array length is equal to
AES_BLOCK_SIZE Thereby you have a cleaner separation of concerns.
Paul.
signature.asc
Description: Message signed with OpenPGP using GPGMail
