RFR: 8327818: Implement Kerberos debug with sun.security.util.Debug [v7]

[Sean Coffey]

On Thu, 14 Mar 2024 20:30:58 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Change `Krb5LoginModule` debugging to use `sun.security.util.Debug`.
>
> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Mark's comments

Looks good. Few minor comments made. 

the new Debug.of method will need to implement some logic from the decorator debug PR at https://github.com/openjdk/jdk/pull/18084  -- depending on integration order, we can tackle that as needed.

src/java.base/share/classes/sun/security/util/Debug.java line 172:

> 170:      * settings. For example,
> 171:      * {@snippet lang=java:
> 172:      * Map<String,String> settings = loadLoginSettings();

minor nit - needs a space : `Map<String, String>`

src/java.base/share/classes/sun/security/util/Debug.java line 174:

> 172:      * Map<String,String> settings = loadLoginSettings();
> 173:      * String property = settings.get("login");
> 174:      * Debug debug = Debug.of("login", setting);

did you mean to use the `property` variable name here ?

src/java.base/share/classes/sun/security/util/Debug.java line 180:

> 178:      * @return a new Debug object if the property is true
> 179:      */
> 180:     public static Debug of(String option, String property) {

the `property` name is a bit confusing here IMO. Most use cases will be string corresponding to a boolean (or null) - Would a boolean paramater make more sense ? Otherwise. I'd suggest renaming variable to something like `debugEnabled`

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

PR Review: https://git.openjdk.org/jdk/pull/18199#pullrequestreview-1938907146
PR Review Comment: https://git.openjdk.org/jdk/pull/18199#discussion_r1526259059
PR Review Comment: https://git.openjdk.org/jdk/pull/18199#discussion_r1526259909
PR Review Comment: https://git.openjdk.org/jdk/pull/18199#discussion_r1526262473