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