RFR: 8251989: Hex formatting and parsing utility [v5]

[Roger Riggs]

On Mon, 19 Oct 2020 16:35:19 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Roger Riggs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since
>> the last revision:
>>  - Merge branch 'master' into 8251989-hex-formatter
>>  - Test enhancements from Chris Hegarty
>>  - Expanded test coverage and fixed related bugs; Added static imports for TestNG; Corrected declaration of return type of
>>    tohexDigits(Appendable...)
>>  - Merge branch 'master' into 8251989-hex-formatter
>>  - Cleanup of javadoc markup
>>  - Added assertions to testVariableLength and samples
>>  - Merge branch 'master' into 8251989-hex-formatter
>>  - temp updates
>>  - Various code review comments, rename UpperCase and LowerCase methods to match Character, remove unnecessary Class name
>>    qualifications, etc.
>>  - 8251989: Hex formatting and parsing utility
>
> src/java.base/share/classes/java/util/HexFormat.java line 332:
> 
>> 330:         if (s == null) {
>> 331:             StringBuilder sb = new StringBuilder(length *
>> 332:                     (delimiter.length() + prefix.length() + suffix.length()) - delimiter.length());
> 
> should that be:
> length * (delimiter.length() + prefix.length() + suffix.length() + 2) - delimiter.length());
> It seems to me that otherwise, if you have no delimiter, no prefix, and no suffix, then your computed size will be 0,
> which seems wrong.

Agreed, will fix.

> src/java.base/share/classes/java/util/HexFormat.java line 569:
> 
>> 567:         }
>> 568:         for (int i = 0; i < literal.length(); i++) {
>> 569:             if (string.charAt(index + i) != literal.charAt(i)) {
> 
> There should probably be a check to verify that `string` contains at least `index + literal.length()` characters.
> Otherwise AIOBE might be thrown by `string.charAt(index + i)` ?

The method is only called when the precondition `index + literal.length() <= string.length()`.
I'll add a comment and an assert.

-------------

PR: https://git.openjdk.java.net/jdk/pull/482