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