On Sun, 6 Oct 2024 19:33:49 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> As it's a pre-existing issue I'd prefer to keep this one focused on the >> switch-over. How would you model unsigned long values here, though? Sure we >> could read into a `BigInteger` or accept negative values, but to really >> support such overflows we might have to rework a lot of things. >> >> FWIW we already cap some values even lower in practice: >> >> end.centot = (int)centot64; // assume total < 2g > >> How would you model unsigned long values here, though? > > I don't think we should. `9223372036854775807 ` should be enough for everyone > :-) > > It may be worth renaming the method to `get64S` and add a `get64` variant > which either clamps at `LONG.MAX_VALUE` or throws `IllegalArgumentException` > for larger values. Call sites doing custom validation (like > `checkZip64ExtraFieldValues`) could then call `get64S` and check for a > negative long. > > But that's food for another PR. Renaming to `get64S` is reasonable to be internally consistent. Updated. Improving validation of data in such 64-bit fields I'll leave for the future. I think a reasonable stance is to throw in the `check` methods if any such field is negative, at least for some of these fields. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21377#discussion_r1789266234
