In fact, I had thought about throwing an exception when the value is not a supported one, but decided not to do that for compatibility reasons. The old Config::getBooleanValue() has never thrown an exception. We have other methods doing similarly (say, getIntValue()r returns Integer.MIN_VALUE is there is a NumberFormatException).
--Max On Jan 29, 2014, at 5:46, Sean Mullan <[email protected]> wrote: > On 01/28/2014 03:53 AM, Wang Weijun wrote: >> Please review the fix at >> >> http://cr.openjdk.java.net/~weijun/8029995/webrev.00/ >> >> The supported boolean values in this fix cover what MIT krb5 does and >> we also added 'f'. >> >> The old getBooleanValue() method returns true for “true” and false >> otherwise but the new method returns null if the value is not >> supported. I’ve carefully changed how the method is called to ensure >> maximum compatibility, but there is still one left: >> >> We support DNS lookup for realm name by default, which means we do it >> if dns_lookup_realm is not set to false, or when unset, if >> dns_fallback is not set to false. Before this change, when >> dns_lookup_realm is set to “unknown”, it means false so DNS lookup is >> not performed. After this change, it’s equivalent to dns_lookup_realm >> unset, and dns_fallback is used. I think the current behavior is >> better than the old one. > > I agree, but since it is a behavior change, it should be specified in the CCC > and release notes. > > Fix looks fine otherwise. > > --Sean
