On Sat, 30 Mar 2024 03:47:10 GMT, Liam Miller-Cushon <cus...@openjdk.org> wrote:
> I think that's similar idea to one of the alternatives I mentioned earlier, > won't that allocate for every central directory entry? Ok. Re-reading your earlier comment more closely, I see you mentioned allocating memory option. The allocation of the buffer for reading extra fields is only done when needed (if `cen_offset` is `ZIP64_MAGICVAL` for the validate_lochdr case). I think I'm missing the part about the "allocate for every central directory entry" with the approach. Could you please elaborate a bit more? > This callsite has already read the data we need into a buffer, if we end up > doing something like that it might be better to do the allocation only for > the call site in `validate_lochdr`, and have the shared helper take the > pointer to the buffer instead of having a duplicate read. > > It seems like there are some tradeoffs here, I can definitely restructure > this, but I'd also like to get some feedback from a core-libs owner on the > overall approach before iterating too much more. Yeah, agreed. It's a good idea to have someone work closely with ZIP and have more insights on central directory structure to take a look at this change now. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18479#discussion_r1545465731
