RFR: 8261949: fileStream::readln returns incorrect line string [v2]

[Yang Yi]

On Fri, 19 Feb 2021 01:22:51 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> src/hotspot/share/utilities/ostream.cpp line 593:
>> 
>>> 591:     ret = ::fgets(data, count, _file);
>>> 592:     // Get rid of annoying \n char only if it presents, it works for Posix
>>> 593:     // and Windows since the last character of these systems is always '\n'
>> 
>> s/of these/on these/
>> Please end the comment with a period.
>
> I suggest simply:
> 
> // Get rid of \n char if it is present

Changed.

>> src/hotspot/share/utilities/ostream.cpp line 595:
>> 
>>> 593:     // and Windows since the last character of these systems is always '\n'
>>> 594:     if (data[::strlen(data)-1] == '\n') {
>>> 595:       data[::strlen(data)-1] = '\0';
>> 
>> Perhaps:
>>     size_t last_char = ::strlen(data) - 1;
>>     if (last_char >= 0 && data[last_char] == '\n') {
>>       data[last_char] = '\0';
>
> @dcubed-ojdk size_t is unsigned so by definition always >= 0. :) I suggest:
> `size_t len = ::strlen(data);`
> `if (len > 0 && data[len - 1] == '\n') {`
> `  data[len - 1] = '\0';`
> `}`

Make sense. Changed.

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

PR: https://git.openjdk.java.net/jdk/pull/2626