Hi Joe, to me this looks good.
Did you already remove the cleanups from http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see a lot of them any more... A few minor points: It seems you still have left some debugging code in test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java: 59 while (xmlStreamReader.hasNext()) { 60 int event = xmlStreamReader.next(); 61 if (event == XMLStreamConstants.START_ELEMENT) { 62 if (xmlStreamReader.getLocalName().equals("body"))// && bMessage) -> remove the && bMessage) 63 { 64 String elementText = xmlStreamReader.getElementText(); 65 //System.out.println("elementText=" + elementText + "EndOfElementText"); -> the commented System.out.println statement should be removed at all, I suggest 66 // fail if elementText contains "</body>" 67 Assert.assertTrue(!elementText.contains("</body>"), "Fail: elementText contains </body>"); 68 } 69 } 70 } Other than that I'm wondering if the 80 chars line limit shall be held which is not completely true in StreamReaderTest.java. But I know how difficult that is, while keeping the code still readable. Especially with the long speaking Class and method names ;-) Best regards Christoph > -----Original Message----- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf > Of Joe Wang > Sent: Mittwoch, 30. November 2016 22:21 > To: core-libs-dev@openjdk.java.net > Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt > content when size of element is > 8192 > > Hi, > > Please review an one-line fix and a bunch of cleanups. > > The reported issue was caused by a missed setting when the > XMLStreamReader initializes the XML 1.1 scanner, so while the changeset > involved 350 lines, the fix is really just the following: > > XMLStreamReaderImpl.java: > @@ -605,11 +604,12 @@ > ... > + fEntityScanner.registerListener(fScanner); > > > All other changes are cleanups, warnings. And BTW, warnings in the jaxp > repo have come down from 5230 in 2015 to 3265, a result of a bit of > cleanups here and there when we touch those classes. Still a long way to > go, and it shows we may need to have a few dedicated patches. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8167340 > webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ > > Thanks, > Joe
