RFR 8213031: (zipfs) Add support for POSIX file permissions
Langer, Christoph
christoph.langer at sap.com
Tue May 21 15:24:23 UTC 2019
Hi Alan,
Thank you for your comments. Here comes the next update... increasing the turnaround time a little bit ��
http://cr.openjdk.java.net/~clanger/webrevs/8213031.10/
> 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).
Ok, makes sense. I've updated the coding such that the zip's file owner would be the default owner, in case it can be retrieved.
> 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.
I changed the class names, hopefully to your liking.
> 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).
I updated these places. Seems more readable indeed.
> In the same area, the code in initOwner catches UOE but that will always
> be wrapped in PAE.
Fixed.
> > 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.
I have updated the doc in module-info.java quite a bit. Please check.
Is it time to work on the CSR now? How shall we proceed there?
Thanks
Christoph
More information about the core-libs-dev
mailing list