My first webrev review request: 6850337 Hasher.java interprets given option value badly (original) (raw)

Xueming Shen Xueming.Shen at Sun.COM
Mon Jul 20 18🔞02 UTC 2009

Ulf,

I believe the "mb" is designed to be "exclusive". I'm the one to blame, I changed the default mb value from 10 to 11 for my 1.6 Console work, which moved all OEM charsets from the "extended" into the "standard" therefor required a bigger default value, but I FORGOT to update the help message as well. My bad. I guess the better fix for this one is to update the err.println(...). I will be the sponsor if you still want to "fix" this one.

Sherman

btw, (1) The bugid# which I updated the mb from 10 to 11 is #6349456. (2) So far the "convention" is to put the contributor name into the hg commit message not the source file.

Ulf Zibis wrote:

Are you sure this is not done by design and that mb is not in fact meant to be an exclusive rather than inclusive bound? Option usage of Hasher.java says: " -md depth max chain depth (default 3)" " -mb bits max index bits (lg of table size, default 10)" In case of setting -md to x the max chain depth properly results in x, but ... in case of setting -mb to x the max lg of table size inproperly results in x-1 bits, as you can see in -verbose output of the hasher. To ensure, that the default value is processed as it would be 10, variable maxBits is pre-set to 11, to "workaround" the bug. When fixing this bug, any usage of this option should be corrected to x-1 to ensure identical results. I don't think the different boundary behaviour of -md and -mb is done by design. Maybe processing of -md has been wrong too in first time and was corrected, but correction for -mb was overseen, as never used. Maybe you could have a short call to the author Mark Reynolds to ensure this. Possibly he also has knowledge of other usage than in genCharsetProvider.sh. Have you checked that the OpenJDK can still be built with this change in place? I have scanned over - hg.openjdk.java.net/jdk7/tl/jdk/make - hg.openjdk.java.net/jdk7/tl/jdk/src - hg.openjdk.java.net/jdk7/tl/jdk/test The only usage of Hasher.java I've found in: - hg.openjdk.java.net/jdk7/tl/jdk/make/java/nio/genCharsetProvider.sh In this case, the option -mb is not used. I assume, this is the reason, why nobody has experienced this bug before. I have not scanned over - hg.openjdk.java.net/jdk7/tl/hotspot .../langtools etc. as I don't have a hg clone of them. Maybe I should have done this, but I think, it's anyway good that experienced SUN engineer would do that scan too, and I guess, you have better tools to do that. -Ulf

More information about the core-libs-dev mailing list