RFR: 8299825: Move StdoutLog and StderrLog to LogConfiguration

David Holmes dholmes at openjdk.org
Thu Jun 8 12:19:54 UTC 2023


On Thu, 8 Jun 2023 11:52:11 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi,
> 
> Please consider this small cleanup. `LogConfiguration` initializes and handles the lifetime of `StdoutLog` and `StderrLog`, therefore they should logically belong to that class. I leave the visibility as public for the time being, to change it to private would require the gtests to all be part of a fixture.

src/hotspot/share/logging/logConfiguration.cpp line 47:

> 45: 
> 46: LogStdoutOutput* LogConfiguration::LogConfiguration::StdoutLog = nullptr;
> 47: LogStderrOutput* LogConfiguration::LogConfiguration::StderrLog = nullptr;

Why is `LogConfiguration` given twice?

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?

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

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,

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222952755
PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222953544
PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222953970
PR Review Comment: https://git.openjdk.org/jdk/pull/14376#discussion_r1222958545


More information about the hotspot-runtime-dev mailing list