RFR: 8308252: Refactor line-by-line file reading code [v3]

Ioi Lam iklam at openjdk.org
Thu May 18 21:03:58 UTC 2023


On Thu, 18 May 2023 09:50:47 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed typo in comments
>
> src/hotspot/share/utilities/lineReader.cpp line 65:
> 
>> 63: // \n is treated as the line separator.
>> 64: // All occurrences of \r are stripped.
>> 65: char* LineReader::get_line() {
> 
> I would return const char* here, this is an internal buffer.

The caller often breaks the line into multiple tokens (by adding `'\0'`), so it's more useful for the buffer to be modifiable.

> src/hotspot/share/utilities/lineReader.cpp line 76:
> 
>> 74:       if (new_length < _buffer_length) {
>> 75:         // This could happen on 32-bit. On 64-bit, the VM would have exited
>> 76:         // due to OOM before we ever get to here.
> 
> This is scary. I don't like a general utility to use half my address space on 32-bit if bad things happen. I would cap the max. buffer size to something sensible, e.g. 1K or 64K.

I'll add a max size with default to 1MB.

Anyway, if you want to be scared, you should look at GrowableArray, which doubles itself with no upper limit checks. :-)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14025#discussion_r1198299568
PR Review Comment: https://git.openjdk.org/jdk/pull/14025#discussion_r1198298094


More information about the hotspot-dev mailing list