On 09/05/2019 16:16, Langer, Christoph wrote:
Hi Alan,
I've got a new iteration for the zipfs POSIX support and addressed your
concerns: http://cr.openjdk.java.net/~clanger/webrevs/8213031.9/
Thanks. I think you've addressed most of my points. The only thing that
isn't clear is the group owner as I thought we had converged on using
the zip file's group owner. If I read the code correctly then it uses
the file owner (and makes the assumption that defaultOwner is
initialized before initGroup is called).
In passing, the name of the PosixFileAttributeView implementation should
probably be ZipPosixFileAttributeView rather than
ZipFilePosixAttributeViewPosix. Also EntryPosix extends Entry, should
probably be PosixEntry. Not important as these are internal but noticed
them when scanning the changes.
Also in passing, there are several places using
AccessController.doPrivileged that are bit ugly due to the casting. The
doPrivileged methods are awkward to use with lambda expressions (not
your doing) but one approach that I find readable is to separate the
creation of the action, e.g. file the zip file owner it could be:
PrivilegedExceptionAction<UserPrincipal> pa = () -> Files.getOwner(zfpath);
return AccessController.doPrivileged(pa).
In the same area, the code in initOwner catches UOE but that will always
be wrapped in PAE.
Not important to the discussion here of course.
:
I have updated module-info a little bit to reflect the latest changes. Is it
now time to focus on the CSR?
The "For extended POSIX support ..." paragraph has the property name
from a previous iteration so I assume you'll update that. I think it
would be using to put quotes around the names too. It also specifies the
fallback group owner to be the file owner but I think we converged on
use the zip file's group owner where possible.
A small bit of word smiting required but I think this is really close to
writing the CSR.
-Alan
