RFR: 8256828: ostream::print_cr() truncates buffer in copy-through case [v3]

[Thomas Stuefe]

On Tue, 4 Jun 2024 02:09:13 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Clarifies the behaviour of this function in regards to truncation when adding a CR. Ensures a truncation warning is always issued.
>> 
>> Adds unit testing for the specialized cases.
>> 
>> See JBS for discussion of other suggestions.
>> 
>> Testing: - tiers 1-4
>> 
>> Thanks
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   revert constexpr usage

src/hotspot/share/utilities/defaultStream.hpp line 33:

> 31:   friend void ostream_abort();
> 32:  public:
> 33:   class TestSupport;  // Unit test support

Interesting variant of the "friend class for test support". I never thought of this. An advantage is that its name is scoped to the testee class, so one does not have to think up a good name.

src/hotspot/share/utilities/ostream.cpp line 142:

> 140:     warning("outputStream::do_vsnprintf output truncated -- buffer length is " SIZE_FORMAT
> 141:             " bytes but " SIZE_FORMAT " bytes are needed.",
> 142:             add_cr ? buflen + 1 : buflen, required_len + 1);

PrintWarnings defaults to true, right? Ah, I see, you limit to debug builds. Okay.

test/hotspot/gtest/utilities/test_ostream.cpp line 147:

> 145:     va_list ap;
> 146:     va_start(ap, format);
> 147:     const char* res = tty->do_vsnprintf(buf, len, format, ap, add_cr, rlen);

This was confusing at first glance. Any reason you use tty instead of just defaultStream::do_vsnprintf? Or, outputStream::do_vsnprintf?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19512#discussion_r1625318876
PR Review Comment: https://git.openjdk.org/jdk/pull/19512#discussion_r1625331428
PR Review Comment: https://git.openjdk.org/jdk/pull/19512#discussion_r1625319845