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

[Ioi Lam]

On Thu, 18 May 2023 09:30:15 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/cds/classListParser.cpp line 63:
> 
>> 61:   if (!_reader.is_opened()) {
>> 62:     char errmsg[JVM_MAXPATHLEN];
>> 63:     os::lasterror(errmsg, JVM_MAXPATHLEN);
> 
> _reader should buffer errno after the failing OS call. We should not have to rely on os::lasterror() being called right after whatever OS API failed inside the reader. Neither is os::lasterror() necessary, we can just use os::strerror since reader only uses Posix file APIs.

I added `int LineReader::last_errno()`.

> src/hotspot/share/utilities/lineReader.cpp line 30:
> 
>> 28: #include "utilities/lineReader.hpp"
>> 29: 
>> 30: LineReader::LineReader(const char* filename) : _filename(filename), _stream(nullptr) {
> 
> Maybe strdup the file name to be sure? Up to you. We usually just feed literals, so this may be ok.

Done.

> src/hotspot/share/utilities/lineReader.cpp line 44:
> 
>> 42:     }
>> 43:   } else {
>> 44:     _stream = nullptr;
> 
> unnecessary

Removed.

> src/hotspot/share/utilities/lineReader.cpp line 71:
> 
>> 69:   size_t buffer_pos = 0;
>> 70:   int c;
>> 71:   while ((c = getc(_stream)) != EOF) {
> 
> Lets not read individual characters. Lets use fgets or fread() or just plain read(). Preferably the first.

I've reimplemented using fgets().

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14025#discussion_r1201470111
PR Review Comment: https://git.openjdk.org/jdk/pull/14025#discussion_r1201469919
PR Review Comment: https://git.openjdk.org/jdk/pull/14025#discussion_r1201469865
PR Review Comment: https://git.openjdk.org/jdk/pull/14025#discussion_r1201469803