RFR (JAXP) 8029955 : AIOB in XMLEntityScanner.scanLiteral upon parsing literals with > 100 LF chars

[huizhe wang]

Thanks for the comments. I incorporated the suggested changes.

In terms of code format, for small source files, I would just hit the 
source->format button. For big files like this one, it'd unfortunately 
create too much distraction from real changes.

http://cr.openjdk.java.net/~joehw/jdk8/8029955/webrev/

Joe

On 12/17/2013 1:03 PM, Ulf Zibis wrote:
> Also you could increment fCurrentEntity.position at the end of the 
> loop, save 1 "if" and indent correctly with 8 spaces:
> 1166         for (; fCurrentEntity.position<fCurrentEntity.count; 
> fCurrentEntity.position++) {
> 1167             c = fCurrentEntity.ch[fCurrentEntity.position];
> 1168             if ((c == quote &&
> 1169                     (!fCurrentEntity.literal || isExternal)) ||
> 1170                     c == '%' || !XMLChar.isContent(c)) {
> 1171                 break;
> 1172             }
> 1173             if (whiteSpaceInfoNeeded && c == ’\t’) {
> 1174                 storeWhiteSpace(fCurrentEntity.position);
> 1175             }
> 1176         }
>
> -Ulf
>
> On 17.12.2013 20:49, Ulf Zibis wrote:
>> Hi Joe,
>>
>> I would use ’\t’ instead of 0x9, to stay consistent with ahead code:
>> 1175                 if (c == ’\t’) {
>>
>> Lines 1214..1221 could be simpler:
>> 1214         if (whiteSpaceLen >= whiteSpaceLookup.length) {
>> 1215             int [] tmp = new int[whiteSpaceLookup.length*2];
>> 1216             System.arraycopy(whiteSpaceLookup, 0, tmp, 0, 
>> whiteSpaceLookup.length);
>> 1217             whiteSpaceLookup = tmp;
>> 1218         }
>> 1219         whiteSpaceLookup[whiteSpaceLen++] = whiteSpacePos;
>>
>> Or even shorter:
>> 1214         if (whiteSpaceLen >= whiteSpaceLookup.length)
>> 1215             whiteSpaceLookup = Arrays.copyOf(whiteSpaceLookup, 
>> whiteSpaceLookup.length*2);
>> 1216         whiteSpaceLookup[whiteSpaceLen++] = whiteSpacePos;
>>
>> (please insert spaces around if clause, else and after commas.)
>>
>> -Ulf
>>
>>
>> On 16.12.2013 20:31, huizhe wang wrote:
>>> Hi,
>>>
>>> This is a quick fix for a whitespace buffer that was not adjusted 
>>> properly in one of the two cases.  The buffer, whiteSpaceLookup, is 
>>> filled in two cases and adjusted properly the 2nd time. The code is 
>>> moved into a method storeWhiteSpace so that it's shared for the 1st 
>>> case as well.
>>>
>>> Note at line 1175, there is no need to save character 0x20 since all 
>>> whitespace characters will later be replaced with character 0x20.
>>>
>>> webrevs:
>>> http://cr.openjdk.java.net/~joehw/jdk8/8029955/webrev/
>>>
>>> Thanks,
>>> Joe
>>>
>>
>>
>