On Mon, 14 Aug 2023 21:38:02 GMT, Volker Simonis <simo...@openjdk.org> wrote:
>> Hi Volker, >> >> I understand your point and I had done that previously but decided I did >> not like the flow of the code that way which is why I moved the check. I >> prefer to leave it as is. > > I don't think this is a question of "taste" because > `isZip64ExtBlockSizeValid()` suggests that the method will check for *valid* > sizes and to my understanding `0` is not a valid input. This method might > also be called from other places in the future which do not handle the zero > case appropriately. > > In any case, I'm ready to accept this as a case of "Disagree and Commit" :) > but in that case please update at least the comment below to something like > "*..Note we do not need to check blockSize is >= 8 as we know its length is > at least 8 by now*" because "*..from the call to isZip64ExtBlockSizeValid()*" > isn't true any more. I think I agree with Volker that it would be better if isZip64ExtBlockSizeValid continued to return false for block size 0. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15273#discussion_r1294436616
