Still looks fine to me. Thanks, Max
> On May 19, 2020, at 5:36 AM, Valerie Peng <valerie.p...@oracle.com> wrote: > > Updated again due to the merge with Tony's EdDSA change: > > http://cr.openjdk.java.net/~valeriep/8242151/webrev.06 > > Added Ed25519 and Ed448 to KnownOIDs, and rest are just adjustments > accordingly. > > Touched: > > src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdDSAParameters.java > src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java > src/java.base/share/classes/sun/security/x509/AlgorithmId.java > src/java.base/share/classes/sun/security/util/KnownOIDs.java > > Pre-integration Mach5 job is running. > > Thanks, > > Valerie > > On 5/18/2020 11:44 AM, Valerie Peng wrote: >> Great, thanks much for the thorough review~ >> >> Valerie >> >> On 5/15/2020 8:57 PM, Weijun Wang wrote: >>> Well done. Everything looks fine to me. >>> >>> --Max >>> >>>> On May 16, 2020, at 5:47 AM, Valerie Peng <valerie.p...@oracle.com> wrote: >>>> >>>> Hi Max, >>>> >>>> I have updated the webrev >>>> (http://cr.openjdk.java.net/~valeriep/8242151/webrev.05/) to address your >>>> suggestion below. Touched classes are NamedCurve, CurveDB, >>>> ConstraintsParameters, and SunEC. The result of using the single method >>>> looks pretty good - clean and shorter code. :) >>>> >>>>> CurveDB.getNamesByOID is only used in >>>>> ConstraintsParameters.getNamedCurveFromKey(), but we already have a >>>>> NamedCurve there and you can directly use it without converting to >>>>> nc.getObjectId(). >>>>> >>>>> In fact, it looks like nc.getAliases() and nc.getName() are always used >>>>> together. Can we just remove these 2 and add a new method >>>>> nc.getNameAndAliases()? Then there will be no compatibility impact for >>>>> getName() at all! >>>>> >>>> Thanks, >>>> Valerie >>>>
