RFR: 8299825: Move StdoutLog and StderrLog to LogConfiguration [v2]

[Johan Sjölen]

On Thu, 8 Jun 2023 12:12:30 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Remove from header
>>  - Fixes
>
> src/hotspot/share/logging/logConfiguration.cpp line 109:
> 
>> 107: void LogConfiguration::initialize(jlong vm_start_time) {
>> 108:   LogConfiguration::StdoutLog = new LogStdoutOutput();
>> 109:   LogConfiguration::StderrLog = new LogStderrOutput();
> 
> Style nit: we're in a LogConfiguration method so we don't need to specify it again do we?

Yeah, I wanted to be very explicit as there's no `_` prepended, but you're probably right that there's enough context for a reader (and it's a jump-to-def away). Fixed the remaining prepends that are unnecessary.

> src/hotspot/share/logging/logConfiguration.cpp line 427:
> 
>> 425:   // Normally options can't be used to change an existing output
>> 426:   // (parse_log_arguments() will report an error), but we make an exception for
>> 427:   // both LogConfiguration::StdoutLog and LogConfiguration::StderrLog as they're initialized automatically
> 
> Overkill to prefix in the comment

OK, removed.

> src/hotspot/share/logging/logFileStreamOutput.cpp line 36:
> 
>> 34: const char* const LogFileStreamOutput::FoldMultilinesOptionKey = "foldmultilines";
>> 35: LogStdoutOutput* StdoutLog = nullptr;
>> 36: LogStderrOutput* StderrLog = nullptr;
> 
> Surely these had to be declared in a header file somewhere, but I don't see it being removed from a header file,

Ouch, good catch, thank you.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222989496
PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222988096
PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222988171