RFR: 8329728: Read long lines in ClassListParser [v2]

Ioi Lam iklam at openjdk.org
Tue Apr 9 17:27:11 UTC 2024


On Mon, 8 Apr 2024 21:03:52 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @matias9927 and @calvinccheung comments - limit line to 4M. Added gtest cases. Test for class names > 64K
>
> src/hotspot/share/utilities/lineReader.cpp line 51:
> 
>> 49: }
>> 50: 
>> 51: char* LineReader::read_line() {
> 
> Do you think it's worth adding an assert here to make sure the `lineReader `has been initialized?

Done.

> test/hotspot/jtreg/runtime/cds/appcds/customLoader/ClassListFormatA.java line 131:
> 
>> 129:         CDSTestUtils.createArchiveAndCheck(opts)
>> 130:             .shouldContain("Preload Warning: Cannot find " + longName)
>> 131:             .shouldContain("Preload Warning: Cannot find No/Such/ClassABCD");
> 
> Could you add a test that checks a line of max size to test overflows?

As we discussed off-line, I limited the max width to 4M chars and added a few gtest cases for it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1558051727
PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1558051751


More information about the hotspot-dev mailing list