Hi Mandy,
Here is the new webrev - I have added comments for the
string constants as well as for the permissions passed
through to the limited doPrivileged call:
http://cr.openjdk.java.net/~dfuchs/webrev_8150840/webrev.02/index.html
I hope that it makes it more clear that the
java.util.logging.* properties are only used when java.logging
is present, and the jdk.system.logger.* properties when
java.logging is not present.
best regards,
-- daniel
On 14/03/16 15:03, Daniel Fuchs wrote:
Hi Mandy,
On 06/03/16 00:29, Mandy Chung wrote:
On Mar 4, 2016, at 9:05 AM, Daniel Fuchs <daniel.fu...@oracle.com>
wrote:
http://cr.openjdk.java.net/~dfuchs/webrev_8150840/webrev.01/
Looks okay in general.
I’m not a fan of using GetPropertyAction. While it’s convenient as
the class already exists, method refs and anonymous class makes what
it does more explicit at the callsite. No big deal.
In this case I find that it nicely simplifies the code.
Does -Djava.util.logging.SimpleFormatter.format=… have any effect if
java.logging is absent (when used together with jdk.system.logger.level)?
No - that was one of the purpose of splitting the
SimpleConsoleLogger class in two (that is: introducing
the SurrogateLogger subclass).
The java.util.logging.* properties are only used with the
SurrogateLogger - which is only used when java.logging is
present and has no custom configuration and until the
application triggers the initialization of the LogManager.
It’s one of the test cases in SimpleConsoleLoggerTest. I would expect
java.util.logging.* properties are used fro java.util.logging
configuration only.
Yes. That's what happens. The SimpleConsoleLoggerTest verifies that.
I mean it verifies that these properties (java.logging.*) are only
used by the SurrogateLogger subclass, and jdk.system.logger.* are only
used by the (unsubclassed) SimpleConsoleLogger.
Ideally there should be an abstract ConsoleLogger class with two
concrete trivial implementations (SimpleConsoleLogger and
SurrogateLogger) but that would be introducing yet another class
so I left it with the simple single inheritance branch.
JUL_FORMAT_PROP_KEY is defined in SimpleConsoleLogger. If I read it
correctly, it’s only used for the limited doPrivileged.
472 new PropertyPermission(JUL_FORMAT_PROP_KEY,
"read"));
Yes. And in the SurrogateLogger subclass.
I was initially confused what SimpleConsoleLogger is done with
java.util.logging formatting.
I see. Logically it belongs to SurrogateLogger but I didn't want
to trigger the loading of the class if it's not used.
If JUL_FORMAT_PROP_KEY is not referenced anywhere else, perhaps just
remove the constant variable and have a comment to explain this
getSimpleFormat method is shared with JUL?
I'd rather keep the constant since it's used in two places. But adding a
comment to clear up any confusion is certainly a good idea.
best regards,
-- daniel
Mandy
