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

[David Holmes]

On Wed, 10 Apr 2024 17:54:25 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Today the `ClassListParser` has a hard-coded limit of 4096 chars for each line in the CDS class list file. However, it's possible for a line to be much longer than than (64KB for the class name, plus extra information that can include path names, IDs, etc).
>> 
>> I wrote a utility class `LineReader` that automatically allocates a buffer before calling `fgets()`. Hopefully this can be useful for other cases where we call `fgets()` with a fixed buffer size.
>> 
>> Max line width is limited to 4M to simplify testing (and avoid running into corner cases when we approach INT_MAX).
>
> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into 8329728-read-arbitrary-long-lines-in-class-list-parser
>  - @dholmes-ora and @calvinccheung comments
>  - Check class name for valid UTF8 encoding
>  - @matias9927 and @calvinccheung comments - limit line to 4M. Added gtest cases. Test for class names > 64K
>  - 8329728: Read arbitrarily long lines in ClassListParser

Updates look good.

A couple of nits bit changes approved. Thanks

src/hotspot/share/cds/classListParser.cpp line 463:

> 461:     err = "class name too long";
> 462:   } else {
> 463:     assert(Symbol::max_length() < INT_MAX && len < INT_MAX, "must be");

The first half of the assert is redundant as Symbol_max_length is fixed at 64K and will never change.

src/hotspot/share/utilities/lineReader.cpp line 55:

> 53: 
> 54: char* LineReader::read_line() {
> 55:   STATIC_ASSERT(0 < MAX_LEN && MAX_LEN <= INT_MAX);

Given the doubling rule this should check `MAX_LEN <= INT_MAX/2`

src/hotspot/share/utilities/lineReader.cpp line 74:

> 72:         // We have read something in previous loop iteration(s). Return that.
> 73:         // The next call to read_line() will return nullptr to indicate EOF.
> 74:         ++ _line_num;

Style nit: no space between unary operator and its operand

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

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18669#pullrequestreview-1992759649
PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1560107907
PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1560114629
PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1560115794