Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v3]

2024-11-14 Thread Roger Riggs
On Thu, 14 Nov 2024 20:28:31 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v3]

2024-11-14 Thread Sean Mullan
On Thu, 14 Nov 2024 20:28:31 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v3]

2024-11-14 Thread Lance Andersen
On Thu, 14 Nov 2024 20:25:35 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Eirik Bjørsnøs
On Thu, 14 Nov 2024 20:02:50 GMT, Lance Andersen wrote: >> I think both are in the 90-100 range. Line length is a bit subjective, but I >> know that at least in the Security Group, we try to keep code to around 80 >> chars so it is easy to review in a side-by-side diff. It also is visually >>

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v3]

2024-11-14 Thread Eirik Bjørsnøs
> Please review this PR which cleans up security manager related code in > `java.util.zip` and `java.util.jar`: > > * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead of > `GetPropertyAction::privilegedGetProperty` > * `ZipFile` is updated to not call SM::checkRead, SM::

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Lance Andersen
On Thu, 14 Nov 2024 19:59:55 GMT, Sean Mullan wrote: >> Yes, they are 96 and 98 chars long, which means I sympathise with both views >> :) >> >> Unless hearing back from @seanjmullan I'll go with Roger's suggestion of one >> line here. > > I think both are in the 90-100 range. Line length is a

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Sean Mullan
On Thu, 14 Nov 2024 19:58:21 GMT, Eirik Bjørsnøs wrote: >> As long as the line in your ide is around 80 characters or less you are >> good to go. If it is say 100 bytes so you have to scroll, that is when I >> would fold the line. >> >> I think you are OK here > > Yes, they are 96 and 98 cha

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Eirik Bjørsnøs
On Thu, 14 Nov 2024 19:51:33 GMT, Lance Andersen wrote: >> See review comments from @RogerRiggs: >> >> https://github.com/openjdk/jdk/pull/22099#pullrequestreview-2435969401 >> https://github.com/openjdk/jdk/pull/22099#discussion_r1842150753 >> >> where the folding of these lines into one was s

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Lance Andersen
On Thu, 14 Nov 2024 19:45:27 GMT, Eirik Bjørsnøs wrote: >> src/java.base/share/classes/java/util/jar/JarFile.java line 182: >> >>> 180: } >>> 181: RUNTIME_VERSION = >>> Runtime.Version.parse(Integer.toString(runtimeVersion)); >>> 182: String enableMultiRelease = >>> Sys

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Eirik Bjørsnøs
On Thu, 14 Nov 2024 19:30:58 GMT, Sean Mullan wrote: >> Eirik Bjørsnøs has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fold lines for System::getProperty when reading enableMultiRelease and >> inhibitZip64 > > src/java.base/share/classe

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Sean Mullan
On Thu, 14 Nov 2024 13:21:33 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Roger Riggs
On Thu, 14 Nov 2024 13:21:33 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Lance Andersen
On Thu, 14 Nov 2024 13:21:33 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Jaikiran Pai
On Thu, 14 Nov 2024 13:21:33 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which cleans up security manager related code in >> `java.util.zip` and `java.util.jar`: >> >> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead >> of `GetPropertyAction::privilegedGetProp

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Eirik Bjørsnøs
On Thu, 14 Nov 2024 12:37:33 GMT, Roger Riggs wrote: >> Eirik Bjørsnøs has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fold lines for System::getProperty when reading enableMultiRelease and >> inhibitZip64 > > src/java.base/share/classe

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas [v2]

2024-11-14 Thread Eirik Bjørsnøs
> Please review this PR which cleans up security manager related code in > `java.util.zip` and `java.util.jar`: > > * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead of > `GetPropertyAction::privilegedGetProperty` > * `ZipFile` is updated to not call SM::checkRead, SM::

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas

2024-11-14 Thread Sean Mullan
On Thu, 14 Nov 2024 10:18:18 GMT, Eirik Bjørsnøs wrote: > Please review this PR which cleans up security manager related code in > `java.util.zip` and `java.util.jar`: > > * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead of > `GetPropertyAction::privilegedGetProperty`

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas

2024-11-14 Thread Roger Riggs
On Thu, 14 Nov 2024 10:18:18 GMT, Eirik Bjørsnøs wrote: > Please review this PR which cleans up security manager related code in > `java.util.zip` and `java.util.jar`: > > * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead of > `GetPropertyAction::privilegedGetProperty`

Re: RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas

2024-11-14 Thread Alan Bateman
On Thu, 14 Nov 2024 10:18:18 GMT, Eirik Bjørsnøs wrote: > Please review this PR which cleans up security manager related code in > `java.util.zip` and `java.util.jar`: > > * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead of > `GetPropertyAction::privilegedGetProperty`

RFR: 8344179: SecurityManager cleanup in the ZIP and JAR areas

2024-11-14 Thread Eirik Bjørsnøs
Please review this PR which cleans up security manager related code in `java.util.zip` and `java.util.jar`: * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead of `GetPropertyAction::privilegedGetProperty` * `ZipFile` is updated to not call SM::checkRead, SM::checkDelete