Hi Joe,

I agree with Christoph's comments below.


best regards,

-- daniel

On 01/12/16 07:40, Langer, Christoph wrote:
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 

59         while (xmlStreamReader.hasNext()) {
  60             int event = xmlStreamReader.next();
  61             if (event == XMLStreamConstants.START_ELEMENT) {
  62                 if (xmlStreamReader.getLocalName().equals("body"))// && 

-> remove the && bMessage)

  63                 {
  64                     String elementText = xmlStreamReader.getElementText();
  65                     //System.out.println("elementText=" + elementText + 

-> the commented System.out.println statement should be removed at all, I 

  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

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


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:

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


Reply via email to