Looks fine to me. A minor comment about the coding conversions. src/share/classes/sun/security/krb5/KdcComm.java ================================================ 437 if (s == null) return -1;
I would suggest always use braces even for single line statement [1]. if (s == null) { return -1; } Xuelei [1]: http://sim.ivi.co/2014/02/love-to-use-braces-even-for-single-line.html On 5/29/2014 5:38 PM, Wang Weijun wrote: > New webrev at > > http://cr.openjdk.java.net/~weijun/8036779/webrev.01/ > > The value can take the form of a bare non-negative integer in milliseconds, > or a non-negative integer followed by "s" (no space between) in seconds. > > Thanks > Max > > On May 19, 2014, at 21:49, Wang Weijun <weijun.w...@oracle.com> wrote: > >> After some discussion with mit and heimdal lead engineers, I don't want to >> support ms at the moment. mit does not use kdc_timeout at all and heimdal's >> internal presentation is of seconds. >> >> So this is my plan: support "s" but if unspecified treat it as "ms". There >> will be a release notes describing this. This won't automatically fix the >> case for the bug reporter but at least give him a workaround -- use the 's' >> unit for interop. >> >> Does this sound clean? >> >> Thanks >> Max >> >> On May 18, 2014, at 23:12, Xuelei Fan <xuelei....@oracle.com> wrote: >> >>> >>>> On May 18, 2014, at 9:48 PM, chris...@zoulas.com wrote: >>>> >>>> On May 18, 10:06am, weijun.w...@oracle.com (Wang Weijun) wrote: >>>> -- Subject: Re: RFR 8036779: sun.security.krb5.KdcComm interprets >>>> kdc_timeout >>>> >>>> | How about this? I will support "s" and "ms" units ("ms" is not defined >>>> by o= >>>> | ther vendors though). But will still try to be a little smart when there >>>> is= >>>> | no unit. >>>> >>>> Heuristics can make the situation worse; they are difficult to document >>>> and explain to users. But if java is going to support ms, let's coordinate >>>> with heimdal and mit to make them support it too. >>>> >>> +1 >>> >>> Xuelei >>> >>>> christos >>>> >> >