Thanks!

Valerie

On 7/7/2020 9:00 AM, Weijun Wang wrote:
I see. No more comment.

--Max

On Jul 7, 2020, at 11:53 PM, Valerie Peng <valerie.p...@oracle.com> wrote:

Hi Max,

Thanks for your review.

Algorithm is also there, so both work technically. With existing APIs, the only 
way to check for service registration is to call getService(...) and check for 
==. If there were another way to check service registration, then it makes 
sense to store Service object.

Valerie

On 7/7/2020 3:44 AM, Weijun Wang wrote:
On Jul 7, 2020, at 12:33 AM, Valerie Peng <valerie.p...@oracle.com> wrote:

Hi Max,

The suggested fix is not much different than the suggested webrev.
I understand they are mostly the same.

The essential change is to call getService(...) for the returned service in 
Provider.getDefaultSecureRandomService(). The only difference here is using the hardcoded 
type "SecureRandom" vs the one returned by getType() call. Is this what you are 
referring to?
That's not a problem.

I re-write the rest to store String instead of Service as it may seem strange 
why the stored Service is not used but re-queried through getService(...). 
Also, looks cleaner to me this way.
It's good if you like it. I was thinking that the Service object is already 
there (in parseLegacyPut and implRemoveService) and it's simpler to store them 
in the map, and maybe later you can detect whether that Service is registered 
and decide if you can simply return it or a create a new one by calling 
getService().

Thanks,
Max

Thanks,
Valerie
On 7/2/2020 9:05 PM, Weijun Wang wrote:
Hi Valerie,

How about the suggested fix from the bug reporter?

Thanks,
Max

On Jul 3, 2020, at 4:52 AM, Valerie Peng <valerie.p...@oracle.com> wrote:

Hi Max and Sean,

Can you help reviewing this fix for JDK-8248505? This is the followup fix for JDK-8246613 
"Choose the default SecureRandom algo based on registration ordering" which you 
reviewed earlier. Based on the feedback, BCFIPS provider overrides 
putService/getService() calls which does not work well with the fix for JDK-8246613. 
Thus, I adapted to store the SecureRandom algorithm names internally and then return the 
result from getService(...) when Provider.getDefaultSecureRandomService() is called. 
Updated the regression test to include a custom provider which simulates the BCFIPS 
provider behavior.

Bug: https://bugs.openjdk.java.net/browse/JDK-8248505

Webrev: http://cr.openjdk.java.net/~valeriep/8248505/webrev.00/

Thanks,
Valerie


Reply via email to