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 >>>>> >>>>>
