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

[Weijun Wang]

On Fri, 15 Mar 2024 13:04:00 GMT, Sean Coffey <coffeys at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Mark's comments
>
> 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>`

Will add one.

> 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 ?

Ah, yes.

> 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`

I was thinking that it will contain more options like `true+timestamp` later. By that time, we only need to update the `Debug` class to add this feature.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18199#discussion_r1526297798
PR Review Comment: https://git.openjdk.org/jdk/pull/18199#discussion_r1526297546
PR Review Comment: https://git.openjdk.org/jdk/pull/18199#discussion_r1526299579