On Sun, 6 Oct 2024 21:49:08 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>>> 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. > FWIW we already cap some values even lower in practice: > > ``` > end.centot = (int)centot64; // assume total < 2g > ``` I submitted #21384 which adds validation of end.centot and also eliminates this narrowing conversion. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21377#discussion_r1789881954
