Hi Lance, thanks for the quick turnaround. I tried to address your findings. Please find the new webrev here: http://cr.openjdk.java.net/~clanger/webrevs/8222276.3/
> o FindEND() > • I know that endsub and comlen fields are not currently used by Zip FS but > given they are valid fields in the header we should probably add a comment > that they are not currently used which is why they are commented out? > > there is a comment in line 1850 where the members of class END are > declared. Shouldn't that suffice? > > Personally, I would add a comment about the unused members in the > method to make it easier…. Ok, I've added comments. > o line 2061 version(boolean) > • Could you add a comment to describe the method as the changes could be > confusing to someone looking at this code for the 1st time > > addressed in v2, please check > > Definitely better but is still a bit confusing as it says stored =10 and > zip64=stored (and the value is 45 ;-) ), can we tweak it a bit more Check it now 😊 > • lines 2362 and 2462 are commented out but were not previously. Can you > clarify why? > > the variables "pos" and "locPos" aren't used thereafter, so no need to > increment them at these places. Maybe I should delete the lines altogether? > > OK, looking at this again (with extra coffee ;-) ) it makes sense and I would > delete the lines. They are a leftover cut & paste code I suspect Deleted. > Can we move makeParentDirs() closer to buildNodeTree(), perhaps the > same for removeFromTree()? Done. > Can the comment on line 1850 be worded better? I know you are trying to > say that the lines commented out are not used but previously the fields > were all together so being a bit more specific would help those coming later > on to look at this code. Same for line 2025. Also can you add back the > comment that writeCEN writes these fields out with a 0 value as we lost that > comment? I updated this, please check. > line 1937: can you fix the typo “tailing" which should be “trailing” in the > comment. Done. Does it look good now? It runs successfully through our regression testing system. Thanks and best regards Christoph
