On Thu, 12 Aug 2021 21:26:08 GMT, SkateScout
<github.com+919349+skatesc...@openjdk.org> wrote:
> Hi,
> i check Long.java line 1301...1311 and i am not sure if this code is really
> good.
>
> 1. If negative is false the whole part ends with two times the same
> substring and this implies the same error.
>
> 2. If negative is true and we get an error than we can throw the error
> without an second parse step or we can use the substring from the first round.
>
> 3. Also as mentioned above the parseLong(text,radix) should be changed to
> parseLong(seq, beginIndex, endIndex, radix)
> this would avoid at least in the positive case the substring at all.
>
> 4. The same points are with Integer as well.
> try {
> result = parseLong(nm.substring(index), radix);
> result = negative ? -result : result;
> } catch (NumberFormatException e) {
> // If number is Long.MIN_VALUE, we'll end up here. The next line
> // handles this case, and causes any genuine format error to be
> // rethrown.
> String constant = negative ? ("-" + nm.substring(index))
> : nm.substring(index);
> result = parseLong(constant, radix);
> }
The pre-existing logic here is iffy, but I think it is correct.
For Integer: If negative is true, then parsing "2147483648" (Integer.MAX_VALUE
+ 1) would throw, be reparsed as "-2147483648" and then be accepted as
Integer.MIN_VALUE. This is the only case that should be non-exceptional, but it
should also be exceedingly rare in practice. For negative values it improves
the error messages a bit to add the "-" and reparse.
Improving the catch clauses with `parseLong(CharSequence, int, int, int)` and
adding an `if (!negative) throw e` case to the catch could theoretically
improve performance of parsing the MIN_VALUE edge case and repeated decoding of
malformed positive numbers, but these are rare or exceptional cases where we
should favor simplicity over optimal performance
-------------
PR: https://git.openjdk.java.net/jdk/pull/5068
