Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v4]

2023-06-16 Thread Sean Coffey
> New functionality in the -XshowSettings menu to display relevant information 
> about JDK security configuration

Sean Coffey has updated the pull request incrementally with two additional 
commits since the last revision:

 - Refactor out security code to helper class
 - Print aliases. Order Provider type/service output.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14394/files
  - new: https://git.openjdk.org/jdk/pull/14394/files/55a85e38..020a1863

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14394&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14394&range=02-03

  Stats: 350 lines in 3 files changed: 211 ins; 133 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/14394.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14394/head:pull/14394

PR: https://git.openjdk.org/jdk/pull/14394


Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v4]

2023-06-16 Thread Alan Bateman
On Fri, 16 Jun 2023 11:14:38 GMT, Sean Coffey  wrote:

>> New functionality in the -XshowSettings menu to display relevant information 
>> about JDK security configuration
>
> Sean Coffey has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Refactor out security code to helper class
>  - Print aliases. Order Provider type/service output.

src/java.base/share/classes/sun/launcher/LauncherHelper.java line 132:

> 130: ResourceBundle.getBundle(defaultBundleName);
> 131: }
> 132: static PrintStream ostream;

I don't think we want SecuritySettings accessing LauncherHelper's field like 
this. I assume printSecuritySetting is just missing a PrintStream parameter so 
that it can be called to print the security settings to a given stream.

src/java.base/share/classes/sun/launcher/LauncherHelper.java line 276:

> 274: first = false;
> 275: } else { // following lines prefix with indents
> 276: ostream.println(TWOINDENT + s);

I assume these changes are per reverted now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1232121754
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1232121990


Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v4]

2023-06-19 Thread Sean Coffey
On Fri, 16 Jun 2023 11:23:48 GMT, Alan Bateman  wrote:

>> Sean Coffey has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Refactor out security code to helper class
>>  - Print aliases. Order Provider type/service output.
>
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 276:
> 
>> 274: first = false;
>> 275: } else { // following lines prefix with indents
>> 276: ostream.println(TWOINDENT + s);
> 
> I assume these changes are be reverted now.

@AlanBateman  - I was planning to keep this trivial change. Does it not read 
better ?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1234248828


Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v4]

2023-06-19 Thread Alan Bateman
On Fri, 16 Jun 2023 11:23:48 GMT, Alan Bateman  wrote:

>> Sean Coffey has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Refactor out security code to helper class
>>  - Print aliases. Order Provider type/service output.
>
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 276:
> 
>> 274: first = false;
>> 275: } else { // following lines prefix with indents
>> 276: ostream.println(TWOINDENT + s);
> 
> I assume these changes are be reverted now.

> @AlanBateman - I was planning to keep this trivial change. Does it not read 
> better ?

It would be better if the name is shortened to printSecuritySettings and it 
takes an indent parameter, that avoid needing to change the accessibility of 
INDENT.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1234287743