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