[security-dev 01459]: Re: Another code reviewer? (Re: Code review request (was Re: [Fwd: Re: [Fwd: Seeking JDK/Kerberos assistance]]))

Valerie Peng Yu-Ching.Peng at Sun.COM
Thu Dec 10 22:06:22 UTC 2009


One more - perhaps KdcAccessibility class doesn't have to be public.
The rest looks fine.
Thanks,
Valerie

On 12/08/09 19:54, Max (Weijun) Wang wrote:
> Hi Valerie
>
>   Webrev updated:
>
>     http://cr.openjdk.java.net/~weijun/6843127/webrev.01
>
>   1. Add synchronized modifier to all methods
>   2. s/PreferredKDCList/KdcAccessbility/g
>   3. s/goodkdcs/list/g
>
> Hi All
>
>   I need another code reviewer, want to backport it to 6u20.
>
>   The bug is at --
>
>     http://bugs.sun.com/view_bug.do?bug_id=6843127
>
>   The description of the fix is at --
>
>     
> http://cr.openjdk.java.net/~weijun/6843127/webrev.01/src/share/lib/security/java.security.cdiff.html 
>
>
> Thanks
> Max
>
> On Dec 9, 2009, at 8:59 AM, Valerie Peng wrote:
>
>> Hi, Max,
>>
>> Ok, it sounds like there isn't an easy way to centralize the KDC 
>> accessibility policy, timeout, and the number of retries. Let's just 
>> leave it as is then.
>> Your changes generally looks fine and here are my only comments:
>> <KrbKdcReq.java>
>> 1. PreferredKDCList.bads is of type HashSet whose access needs to be 
>> explicitly synchronized?
>> 2. Some nitpicking on naming, it seems somewhat confusing to name the 
>> class "PreferredKDCList" when it includes all kdcs for that specific 
>> realm. Maybe something like "KdcAvailability", "KdcAccessibility", or 
>> "KdcByAvailability", etc. Same goes for the local variable "goodkdcs" 
>> in its list(String) method which actually contains all kdcs for the 
>> specific realm in the end.
>>
>> Thanks,
>> Valerie
>>
>> On 11/22/09 22:10, Max (Weijun) Wang wrote:
> ....
>




More information about the security-dev mailing list