My first webrev review request: 6850337 Hasher.java interprets given option value badly

[Ulf Zibis]

Hi Andrew,

thanks for looking at my request. ....

Am 20.07.2009 02:25, Andrew John Hughes schrieb:
> 2009/7/19 Ulf Zibis <Ulf.Zibis at gmx.de>:
>   
>> Martin, Sherman, Neal,
>>
>> do you like to sponsor and review my CR ?
>>
>> Don't worry, it's very simple and small, best for getting familiar with the
>> workflow.
>>
>> See:
>> https://bugs.openjdk.java.net/show_bug.cgi?id=100090
>>
>> Thanks,
>>
>> Ulf
>>
>>
>>
>>
>>     
>
> As this bug is about a tool used during the build, your mail probably
> wants to be directed to the build list.
>   

Good idea, thanks for forwarding.

> 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