Thanks for the review Tony. Yes - no problems with tests. Copyright updated and changeset pushed.

regards,
Sean.


On 03/08/2018 05:23, Anthony Scarpino wrote:
Looks good. Only comment is update the copyright year. No need to regenerate a 
webrev. Assuming it passes all the tests I say it’s ready to push.

Tony

On Aug 2, 2018, at 5:15 AM, Seán Coffey <sean.cof...@oracle.com> wrote:

Thanks Tony. I was asked off thread to add some comments to help code 
maintenance also.

webrev updated. Hope we're nearly ready to push now.

http://cr.openjdk.java.net/~coffeys/webrev.8207775.v4/webrev/

regards,
Sean.

On 01/08/2018 18:41, Anthony Scarpino wrote:
That looks fine to me.

Tony

On 08/01/2018 08:39 AM, Seán Coffey wrote:
Thanks again for review Tony. I think you raise a good point and should give 
some performance gain.

for line 676: maybe this :

                   byte[] copy = Arrays.copyOf(output, len);
                   if (decrypting) {
                       Arrays.fill(output, (byte) 0x00);
                   }
                   return copy;


perhaps the same again for around line 852 :
                   byte[] copy = Arrays.copyOf(output, len);
                   if (decrypting) {
                       Arrays.fill(output, (byte) 0x00);
                   }
                   return copy;

==

Any other reviews/comments from folks before I submit final build/test & webrev 
?

Regards,
Sean.

On 01/08/18 15:55, Anthony Scarpino wrote:


My only comment is if it makes sense to have the change at 676 to also only 
null out on decrypt?

Otherwise I'm fine with the changes

Tony

On 07/31/2018 02:04 AM, Seán Coffey wrote:
Thanks for review Tony. Comments inline..

On 27/07/18 21:02, Anthony Scarpino wrote:
If we are going to add more, here are two more ton consider:

- It looks like there is another Arrays.copyOf() on doFinal() line 851
Good point.
- doFinal()
at line 897 there might be something that should be done with 'buffer'.  In 
particular as a result of line 963's arraycopy().
Yes - I've identified two areas where we can be proactive about nulling out 
'buffer' contents. That's around the same time where we reset 'buffered' to 0. 
See lines 777 and 967

http://cr.openjdk.java.net/~coffeys/webrev.8207775.v3/webrev/

regards,
Sean.
Tony


On 07/27/2018 08:29 AM, Seán Coffey wrote:
Thanks Tony. If it's alright with you, I'd like to make one more edit for this 
change.

http://cr.openjdk.java.net/~coffeys/webrev.8207775.v2/webrev/

There's a condition where we can null out an array early if we're returning a 
copy. See lines 671-683

Regards,
Sean.

On 26/07/18 17:42, Anthony Scarpino wrote:
On 07/26/2018 07:36 AM, Seán Coffey wrote:
https://bugs.openjdk.java.net/browse/JDK-8207775

Simple enough fix to null out some internal buffers once they're no longer 
required.

webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/

regards,
Sean.

that looks fine..

Tony

Reply via email to