Looks fine, but there are TAB characters. Have you run jcheck? remote: test/sun/security/pkcs/pkcs10/PKCS10AttrEncoding.java:29: Tab character remote: test/sun/security/pkcs/pkcs10/PKCS10AttributeReader.java:52: Tab character remote: test/sun/security/pkcs/pkcs7/PKCS7VerifyTest.java:48: Tab character remote: test/sun/security/pkcs/pkcs7/SignerOrder.java:153: Tab character remote: test/sun/security/pkcs/pkcs8/PKCS8Test.java:51: Tab character
--Max > On Nov 20, 2015, at 1:55 AM, Amanda Jiang <amanda.ji...@oracle.com> wrote: > > Hi Max, > > Please check the webrev below which includes fixes for you previous comments: > http://cr.openjdk.java.net/~amjiang/8048357/webrev.05/ > > Thanks, > Amanda > > On 15/11/16 下午11:55, Weijun Wang wrote: >> Hi Amanda >> >> On 11/17/2015 15:03, Amanda Jiang wrote: >>> http://cr.openjdk.java.net/~amjiang/8048357/webrev.03/ >>> >>>> Why "othervm" for all these tests? >> >> The pkcs10 and pkcs8 tests still uses othervm. Is that necessary? I see no >> VM static change made by these tests. >> >>>> PKCS8Test.java: >>>> >>>> - sun.security.x509 in @modules? >>> This test uses "sun.misc.HexDumpEncoder", so that's why security.x509 is >>> in @modules. >> >> I was asking about sun.security.x509, not sun.misc. >> >>>> >>>> pkcs7/*.java: >>>> >>>> Please take a look at Bernd Eckenfels's comments on Aug 21. I fully agree >>>> with him. >> >> In PKCS7VerifyTest.java, you are still using is.available() to get the size >> of FILEPATH + args[1]. >> >> In SignerOrder.java, can you explain what the contents of derString1 and >> derString2 means? You seem to be choosing some specially crafted DER-encoded >> bytes but I don't know the reason. The names and the contents just keep me >> guessing what they are. >> >> Everything else is fine. >> >> Thanks >> Max >