date:20220218

On Fri, 18 Feb 2022 20:54:24 GMT, Tyler Steele  wrote:

>> FileEncodingTest expects all non-Windows platforms will have 
>> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
>> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
>> 
>> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
>> `Charset.defaultCharset().name()` to equal 
>> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. 
>> From JEP-400: "... if file.encoding is set to COMPAT on the command line, 
>> then the run-time value of file.encoding will be the same as the run-time 
>> value of native.encoding...". So one way to resolve this is to choose the 
>> value for each system from the native.encoding property.
>> 
>> With these changes however, my test systems continue to fail. 
>> 
>> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
>> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> 
>> Note that the expected value is populated from native.encoding.
>> 
>> This implies more work to be done. It looks to me that some modification to 
>> java_props_md.c may be needed to ensure that the System properties for 
>> native.encoding return [canonical 
>> names](http://www.iana.org/assignments/character-sets). 
>> 
>> ---
>> 
>> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
>> in the test explicitly, as was done for the Windows expected encoding prior 
>> to this proposed change. The main advantage to this alternative is that it 
>> is quick and easy, but the disadvantages are that it fails to test that 
>> COMPAT behaves as specified in JEP-400, and the approach does not scale well 
>> if it happens that other systems require other cases. I wonder if this is 
>> the reason non-English locals are excluded by the test.
>> 
>> Proceeding with this change and the work implied by the new failures it 
>> highlights goes beyond the scope of what I thought was a simple testbug. So 
>> I'm opening this up for some comments before proceeding down the rabbit hole 
>> of further changes. If there is generally positive support for this 
>> direction I'm happy to make the modifications necessary to populate 
>> native.encoding with canonical names. As I am new to OpenJDK, I am 
>> especially looking to ensure that changing the value returned by 
>> native.encoding will not have unintended consequences elsewhere in the 
>> project.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixes copyright header error

> /label remove auto
> 
> Automatic integration is not appropriate and should not be used except in 
> time sensitive cases. It can preempt other reviewers from having a chance to 
> review and comment.

Ok. I will avoid using it in the future. I don't have commiter rights, so I was 
just looking to signal that I am happy for this change to be integrated when it 
has been reviewed. It seemed to fall under the 'benign changes' category of the 
`/integrate` command's documentation.

If there is other feedback, I am happy to incorporate it 

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v7]

On Fri, 18 Feb 2022 20:54:24 GMT, Tyler Steele  wrote:

>> FileEncodingTest expects all non-Windows platforms will have 
>> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
>> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
>> 
>> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
>> `Charset.defaultCharset().name()` to equal 
>> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. 
>> From JEP-400: "... if file.encoding is set to COMPAT on the command line, 
>> then the run-time value of file.encoding will be the same as the run-time 
>> value of native.encoding...". So one way to resolve this is to choose the 
>> value for each system from the native.encoding property.
>> 
>> With these changes however, my test systems continue to fail. 
>> 
>> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
>> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> 
>> Note that the expected value is populated from native.encoding.
>> 
>> This implies more work to be done. It looks to me that some modification to 
>> java_props_md.c may be needed to ensure that the System properties for 
>> native.encoding return [canonical 
>> names](http://www.iana.org/assignments/character-sets). 
>> 
>> ---
>> 
>> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
>> in the test explicitly, as was done for the Windows expected encoding prior 
>> to this proposed change. The main advantage to this alternative is that it 
>> is quick and easy, but the disadvantages are that it fails to test that 
>> COMPAT behaves as specified in JEP-400, and the approach does not scale well 
>> if it happens that other systems require other cases. I wonder if this is 
>> the reason non-English locals are excluded by the test.
>> 
>> Proceeding with this change and the work implied by the new failures it 
>> highlights goes beyond the scope of what I thought was a simple testbug. So 
>> I'm opening this up for some comments before proceeding down the rabbit hole 
>> of further changes. If there is generally positive support for this 
>> direction I'm happy to make the modifications necessary to populate 
>> native.encoding with canonical names. As I am new to OpenJDK, I am 
>> especially looking to ensure that changing the value returned by 
>> native.encoding will not have unintended consequences elsewhere in the 
>> project.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixes copyright header error

Looks good.

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v6]

On Fri, 18 Feb 2022 20:04:25 GMT, Naoto Sato  wrote:

>> Tyler Steele has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Makes sm changes requested by Naoto
>
> test/jdk/java/lang/System/FileEncodingTest.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> 
> The year should be `2021, 2022`. (year of inception must be preserved)

Oops. Thanks for catching that.

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v7]

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Tyler Steele has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixes copyright header error

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7525/files
  - new: https://git.openjdk.java.net/jdk/pull/7525/files/2a8651b8..ac38c8f3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7525=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7525=05-06

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7525/head:pull/7525

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: Accessing the BCI of Throwable's StackTraceElement

2022-02-18 Thread Mandy Chung


On 2/18/22 3:42 AM, Eirik Bjørsnøs wrote:

Hi,

Currently, StackWalker.StackFrame::getByteCodeIndex provides a way to get
the BCI of stack frames during a stack walk.

Similarly, it might be useful to get the BCI when inspecting a Throwable's
StackTraceElements. This does however not seem possible, given that
StackTraceElement currently does not expose the BCI. (It exposes the
lineNumber, but that's not useful for my current use case).

On another note, it seems like calling Throwable.getStackTrace() would be a
wasteful way to just find the BCI of the top stack frame. Would be really
nice if one could do a StackWalk with a Throwable as input, which I imagine
should work a lot more efficiently.

(My use case here is to attribute any exception to a BCI in the same method
such that I don't need to instrument code to track the BCI myself).

So I guess I have two questions:

1: Would it be reasonable to add a getByteCodeIndex method to
StackTraceElement? Or perhaps even make StackTraceElement implement
StackWalker.StackFrame?


It's possible to consider adding a method in StackTraceElement to return 
BCI but I think having StackWalker to take a Throwable may be a better 
solution.



2: Could the StackWalker API be suitable for walking not just the stack of
current threads, but also the stacks of Throwables? Was this discussed
during the development of StackWalker for Java 9?


Yes it's tracked by JDK-8189752 [1] and one idea is to provide a way to 
traverse the backtrace of a Throwable.


[1] https://bugs.openjdk.java.net/browse/JDK-8189752

Re: Proposed JEP: Safer Process Launch by ProcessBuilder and Runtime.exec

2022-02-18 Thread Raffaello Giulietti

Hello,

to overcome some of the problems with parsing and generating Windows
command lines, I implemented two classes [1] that attempt to provide a
more sophisticated solution. To be clear, they do not create processes
or launch programs. They only serve as a parser and an "escaper".

Currently, they are completely outside the OpenJDK codebase to avoid
interfering with the current behavior. The intent is to have a concrete
basis for a more thorough discussion and some code to experiment with.
Later, the code can be integrated into OpenJDK if so desired.

Both classes perform a straightforward, one-pass left-to-right
processing (each character is read only once) without back-patching.
They only make use String, StringBuilder and ArrayList.

Two important technical aspects must be kept in mind when later using
the outcomes of these classes to start new processes on Windows. Both
are related in the interplay between the Windows function
CreateProcess() [2] and the C/C++ runtime [3]:

* A program can parse the command line as it deems useful. There are no
hard rules, only conventions. These classes assume that the program
denoted on the command line will perform parsing as done by the Windows
C/C++ runtime conventions [3]. If this assumption is invalid, there's no
point in using these classes.

* In particular, the "shell" cmd.exe parses the command line in a
different way. While not currently exposed in these classes, it would be
easy to add a specific parser and escaper for cmd.exe as well.

* Absent the application name, the initial section of the command line
passed to CreateProcess() is parsed by it to locate the program to
launch. The way it parses the program part when it is unquoted is too
cumbersome and depends on the content of the filesystem [2]. Trying to
re-implement this parsing would introduce a potential source of troubles
that could later lead in launching an unintended program. Thus, for
simplification and caution, these classes assume that the program part
is always quoted, throwing otherwise.

Greetings
Raffaello

[1] https://github.com/rgiulietti/experiments
[2]
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa
[3]
https://docs.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments

On 2022-01-20 19:05, Roger Riggs wrote:
A JEP to Improve safety of process launch by ProcessBuilder and
Runtime.exec on Windows[1].

Argument encoding errors have been problematic on Windows systems due to
improperly quoted command arguments.

The idea is to tighten up quoting and encoding of command line arguments.

Comments appreciated, Roger

[1] https://bugs.openjdk.java.net/browse/JDK-8263697

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v6]

On Fri, 18 Feb 2022 19:50:33 GMT, Tyler Steele  wrote:

>> FileEncodingTest expects all non-Windows platforms will have 
>> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
>> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
>> 
>> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
>> `Charset.defaultCharset().name()` to equal 
>> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. 
>> From JEP-400: "... if file.encoding is set to COMPAT on the command line, 
>> then the run-time value of file.encoding will be the same as the run-time 
>> value of native.encoding...". So one way to resolve this is to choose the 
>> value for each system from the native.encoding property.
>> 
>> With these changes however, my test systems continue to fail. 
>> 
>> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
>> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> 
>> Note that the expected value is populated from native.encoding.
>> 
>> This implies more work to be done. It looks to me that some modification to 
>> java_props_md.c may be needed to ensure that the System properties for 
>> native.encoding return [canonical 
>> names](http://www.iana.org/assignments/character-sets). 
>> 
>> ---
>> 
>> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
>> in the test explicitly, as was done for the Windows expected encoding prior 
>> to this proposed change. The main advantage to this alternative is that it 
>> is quick and easy, but the disadvantages are that it fails to test that 
>> COMPAT behaves as specified in JEP-400, and the approach does not scale well 
>> if it happens that other systems require other cases. I wonder if this is 
>> the reason non-English locals are excluded by the test.
>> 
>> Proceeding with this change and the work implied by the new failures it 
>> highlights goes beyond the scope of what I thought was a simple testbug. So 
>> I'm opening this up for some comments before proceeding down the rabbit hole 
>> of further changes. If there is generally positive support for this 
>> direction I'm happy to make the modifications necessary to populate 
>> native.encoding with canonical names. As I am new to OpenJDK, I am 
>> especially looking to ensure that changing the value returned by 
>> native.encoding will not have unintended consequences elsewhere in the 
>> project.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Makes sm changes requested by Naoto

test/jdk/java/lang/System/FileEncodingTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

The year should be `2021, 2022`. (year of inception must be preserved)

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v7]

> Hi all,
> 
> Please review the attached patch to address
> 
> - That JarFile::getInputStream did not check for a null ZipEntry passed as a 
> parameter
> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an 
> unexpected exception occurs
> 
> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar 
> test runs
> 
> Best
> Lance

Lance Andersen has updated the pull request incrementally with one additional 
commit since the last revision:

  remove extra '}'

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7348/files
  - new: https://git.openjdk.java.net/jdk/pull/7348/files/818c2857..839d99f7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7348=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7348=05-06

  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7348.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v6]

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Tyler Steele has updated the pull request incrementally with one additional 
commit since the last revision:

  Makes sm changes requested by Naoto

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7525/files
  - new: https://git.openjdk.java.net/jdk/pull/7525/files/56f01452..2a8651b8

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7525=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7525=04-05

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7525/head:pull/7525

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v6]

> Hi all,
> 
> Please review the attached patch to address
> 
> - That JarFile::getInputStream did not check for a null ZipEntry passed as a 
> parameter
> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an 
> unexpected exception occurs
> 
> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar 
> test runs
> 
> Best
> Lance

Lance Andersen has updated the pull request incrementally with one additional 
commit since the last revision:

  remove trailing space

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7348/files
  - new: https://git.openjdk.java.net/jdk/pull/7348/files/490c986d..818c2857

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7348=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7348=04-05

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7348.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v4]

On Fri, 18 Feb 2022 17:20:26 GMT, Alan Bateman  wrote:

> > If you feel there is still something lacking for documentation, I can 
> > certainly make another pass clarify/add it, but I tried to cover the steps 
> > (but I also understand what might be obvious to me might not be as obvious 
> > as I thought).
> 
> Yes, I think clear instructions are important to have for tests like this. It 
> doesn't need much but what you know is scattered across a method and a 
> comment on a byte array, just not clear/easy.

Ok, thank you for the feedback.  I just pushed a change with additional 
comments on the jar creation which hopefully will address your input above.

-

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v5]

> Hi all,
> 
> Please review the attached patch to address
> 
> - That JarFile::getInputStream did not check for a null ZipEntry passed as a 
> parameter
> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an 
> unexpected exception occurs
> 
> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar 
> test runs
> 
> Best
> Lance

Lance Andersen has updated the pull request incrementally with one additional 
commit since the last revision:

  Add additional comments describing how the jars are created

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7348/files
  - new: https://git.openjdk.java.net/jdk/pull/7348/files/d5cf8db8..490c986d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7348=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7348=03-04

  Stats: 29 lines in 1 file changed: 28 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7348.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v5]

On Fri, 18 Feb 2022 19:06:28 GMT, Tyler Steele  wrote:

>> FileEncodingTest expects all non-Windows platforms will have 
>> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
>> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
>> 
>> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
>> `Charset.defaultCharset().name()` to equal 
>> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. 
>> From JEP-400: "... if file.encoding is set to COMPAT on the command line, 
>> then the run-time value of file.encoding will be the same as the run-time 
>> value of native.encoding...". So one way to resolve this is to choose the 
>> value for each system from the native.encoding property.
>> 
>> With these changes however, my test systems continue to fail. 
>> 
>> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
>> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> 
>> Note that the expected value is populated from native.encoding.
>> 
>> This implies more work to be done. It looks to me that some modification to 
>> java_props_md.c may be needed to ensure that the System properties for 
>> native.encoding return [canonical 
>> names](http://www.iana.org/assignments/character-sets). 
>> 
>> ---
>> 
>> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
>> in the test explicitly, as was done for the Windows expected encoding prior 
>> to this proposed change. The main advantage to this alternative is that it 
>> is quick and easy, but the disadvantages are that it fails to test that 
>> COMPAT behaves as specified in JEP-400, and the approach does not scale well 
>> if it happens that other systems require other cases. I wonder if this is 
>> the reason non-English locals are excluded by the test.
>> 
>> Proceeding with this change and the work implied by the new failures it 
>> highlights goes beyond the scope of what I thought was a simple testbug. So 
>> I'm opening this up for some comments before proceeding down the rabbit hole 
>> of further changes. If there is generally positive support for this 
>> direction I'm happy to make the modifications necessary to populate 
>> native.encoding with canonical names. As I am new to OpenJDK, I am 
>> especially looking to ensure that changing the value returned by 
>> native.encoding will not have unintended consequences elsewhere in the 
>> project.
>
> Tyler Steele has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR. The pull request contains one new 
> commit since the last revision:
> 
>   Changes FileEncodingTest to include hardcoded value for AIX

Changes look good.
Nit: Please add the issue # to the `@bug` tag, and modify the copyright year to 
`2021, 2022`.

-

Changes requested by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v5]

On Fri, 18 Feb 2022 19:06:28 GMT, Tyler Steele  wrote:

>> FileEncodingTest expects all non-Windows platforms will have 
>> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
>> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
>> 
>> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
>> `Charset.defaultCharset().name()` to equal 
>> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. 
>> From JEP-400: "... if file.encoding is set to COMPAT on the command line, 
>> then the run-time value of file.encoding will be the same as the run-time 
>> value of native.encoding...". So one way to resolve this is to choose the 
>> value for each system from the native.encoding property.
>> 
>> With these changes however, my test systems continue to fail. 
>> 
>> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
>> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
>> 
>> Note that the expected value is populated from native.encoding.
>> 
>> This implies more work to be done. It looks to me that some modification to 
>> java_props_md.c may be needed to ensure that the System properties for 
>> native.encoding return [canonical 
>> names](http://www.iana.org/assignments/character-sets). 
>> 
>> ---
>> 
>> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
>> in the test explicitly, as was done for the Windows expected encoding prior 
>> to this proposed change. The main advantage to this alternative is that it 
>> is quick and easy, but the disadvantages are that it fails to test that 
>> COMPAT behaves as specified in JEP-400, and the approach does not scale well 
>> if it happens that other systems require other cases. I wonder if this is 
>> the reason non-English locals are excluded by the test.
>> 
>> Proceeding with this change and the work implied by the new failures it 
>> highlights goes beyond the scope of what I thought was a simple testbug. So 
>> I'm opening this up for some comments before proceeding down the rabbit hole 
>> of further changes. If there is generally positive support for this 
>> direction I'm happy to make the modifications necessary to populate 
>> native.encoding with canonical names. As I am new to OpenJDK, I am 
>> especially looking to ensure that changing the value returned by 
>> native.encoding will not have unintended consequences elsewhere in the 
>> project.
>
> Tyler Steele has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR. The pull request contains one new 
> commit since the last revision:
> 
>   Changes FileEncodingTest to include hardcoded value for AIX

The test now passes on AIX, and Linux/Z. So I believe this change can be merged 
once reviewed.

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v20]

On Fri, 18 Feb 2022 18:53:43 GMT, XenoAmess  wrote:

>> 8281631: HashMap copy constructor and putAll can over-allocate table
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   revert unrelated changes and add it to ProblemList.txt
>   
>   https://bugs.openjdk.java.net/browse/JDK-8282120

I found some other interesting thing using the rewritten test (about 
IdentityHashMap, also memory related problem), but would investigate it 
tomorrow.
Too late in my time zone today.
Would inform you once I have finished my investigation.

-

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v5]

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Tyler Steele has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  Changes FileEncodingTest to include hardcoded value for AIX

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7525/files
  - new: https://git.openjdk.java.net/jdk/pull/7525/files/9341ecb7..56f01452

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7525=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7525=03-04

  Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7525/head:pull/7525

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v19]

On Fri, 18 Feb 2022 18:32:31 GMT, Stuart Marks  wrote:

> Sigh. (I'm sighing at the author of the 
> Enum/ConstantDirectoryOptimalCapacity.java test, not you.) What a mess. See 
> https://bugs.openjdk.java.net/browse/JDK-8282120 which I just filed. The 
> broken test and the OptimalCapacity utilities are mostly useless, so the 
> change to that test and Class.java should be removed this PR, and the test 
> added to the Problem List so it doesn't get run anymore. See 
> test/jdk/ProblemList.txt and add an entry for the now-failing test, with a 
> reference to 8282120.

Got it. I would have a try.

> I don't think I'll have time to look at the junit rewrite and to run this 
> through our internal build/test system today, so sorry, this may need to wait 
> a week.

OK, see you next week~

-

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v4]

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Tyler Steele has updated the pull request incrementally with one additional 
commit since the last revision:

  Minor fixes: Removes unused Booleans, adds varname

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7525/files
  - new: https://git.openjdk.java.net/jdk/pull/7525/files/ff2918d6..9341ecb7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7525=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7525=02-03

  Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7525/head:pull/7525

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v20]

> 8281631: HashMap copy constructor and putAll can over-allocate table

XenoAmess has updated the pull request incrementally with one additional commit 
since the last revision:

  revert unrelated changes and add it to ProblemList.txt
  
  https://bugs.openjdk.java.net/browse/JDK-8282120

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7431/files
  - new: https://git.openjdk.java.net/jdk/pull/7431/files/aa599698..182c22d7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=19
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=18-19

  Stats: 4 lines in 3 files changed: 1 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7431.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v3]

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Tyler Steele has updated the pull request incrementally with one additional 
commit since the last revision:

  Changes FileEncodingTest to include hardcoded value for AIX

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7525/files
  - new: https://git.openjdk.java.net/jdk/pull/7525/files/83b6e4bc..ff2918d6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7525=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7525=01-02

  Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7525/head:pull/7525

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding [v2]

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Tyler Steele has refreshed the contents of this pull request, and previous 
commits have been removed. Incremental views are not available.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7525/files
  - new: https://git.openjdk.java.net/jdk/pull/7525/files/fd06a608..83b6e4bc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7525=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7525=00-01

  Stats: 8 lines in 1 file changed: 4 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7525/head:pull/7525

PR: https://git.openjdk.java.net/jdk/pull/7525

Withdrawn: 8282042: [testbug] FileEncodingTest.java depends on default encoding

On Thu, 17 Feb 2022 22:50:37 GMT, Tyler Steele  wrote:

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding

On Thu, 17 Feb 2022 22:50:37 GMT, Tyler Steele  wrote:

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

Thanks for your feedback Naoto.

I agree it's a little odd to test the way I proposed above, as it introduces 
uncertainty as you mentioned, as well as other issues like both native.encoding 
and Charsets.defaultCharset() being wrong, but being wrong in the same way. The 
main part of testing this way was the quoted line from JEP-400 (of which I 
recognize you are an author). Maybe I'm being too literal; in my testing the 
encodings match, even if the names are aliases of the ones I expect. In 
addition, you have a good point about the purpose of the COMPAT flag being 
compatibility. I agree that it's not really appropriate to change the values of 
native.encoding to the canonical ones.

I was feeling torn between the proposed option and alternative, and your 
feedback definitely sways me towards the alternative. I'll change this PR to 
simply add an exception to the test for AIX.

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v19]

2022-02-18 Thread Stuart Marks

On Fri, 18 Feb 2022 15:29:25 GMT, XenoAmess  wrote:

>> 8281631: HashMap copy constructor and putAll can over-allocate table
>
> XenoAmess has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - migrate to junit
>  - change threshold

Sigh. (I'm sighing at the author of the 
Enum/ConstantDirectoryOptimalCapacity.java test, not you.) What a mess. See 
https://bugs.openjdk.java.net/browse/JDK-8282120 which I just filed. The broken 
test and the OptimalCapacity utilities are mostly useless, so the change to 
that test and Class.java should be removed this PR, and the test added to the 
Problem List so it doesn't get run anymore. See test/jdk/ProblemList.txt and 
add an entry for the now-failing test, with a reference to 8282120.

I don't think I'll have time to look at the junit rewrite and to run this 
through our internal build/test system today, so sorry, this may need to wait a 
week.

-

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: JDK-17: Wndows jpackage destination directory not writable

2022-02-18 Thread Alexey Semenyuk


Hi Sverre,

Interesting, I don't see changes in jpackage code related to the issue. 
In particular jdk.jpackage.internal.IOUtils.writableOutputDir() function 
is the same in JDK14, JDK17, and mainline.


- Alexey

On 2/18/2022 8:31 AM, Sverre Moe wrote:

I executed our JDK11 docker image, which works fine with JDK11 and JDK14
(for jpackage support).
Then I installed the JDK17 MSI package, changed JAVA_HOME and PATH.
Building our application now with JDK17 it still cannot write to the
"build/native" jpackage output directory.

Leads me to conclude something has changed in jpackage between JDK14 and
JDK17.
I modified my gradle configuration, to use jpackage from JDK14 when
packaging my JDK17 built application.
It works using JDK14 jpackage to package our JDK17 application.

Using the JDK17 jpackage directly in Windows works fine. It is only in the
Docker container that it does not work.

/Sverre



tir. 5. okt. 2021 kl. 11:55 skrev Sverre Moe:


I ran cacls after the failed jpackage.

C:\temp\my-javafx-application>cacls build
C:\temp\my-javafx-application\build F
  CREATOR OWNER:(OI)(CI)(IO)F
  R
  CREATOR GROUP:(OI)(CI)(IO)R
  Everyone:(OI)(CI)R

C:\temp\my-javafx-application>cacls build\native
C:\temp\my-javafx-application\build\native F
 CREATOR OWNER:(OI)(CI)(IO)F
 R
 CREATOR GROUP:(OI)(CI)(IO)R
 Everyone:(OI)(CI)R


As cacls was deprecated it suggested to use icacls instead:

C:\temp\my-javafx-application>icacls build
build S-1-5-21-406077803-2019195645-689573112-1003:(I)(F)
   CREATOR OWNER:(I)(OI)(CI)(IO)(F)
   S-1-5-21-406077803-2019195645-689573112-513:(I)(RX)
   CREATOR GROUP:(I)(OI)(CI)(IO)(RX)
   Everyone:(I)(OI)(CI)(RX)

Successfully processed 1 files; Failed processing 0 files

C:\temp\my-javafx-application>icacls build\native
build\native S-1-5-83-1-1537791174-1084404783-2478187907-2577323605:(I)(F)
  CREATOR OWNER:(I)(OI)(CI)(IO)(F)
  S-1-5-83-0:(I)(RX)
  CREATOR GROUP:(I)(OI)(CI)(IO)(RX)
  Everyone:(I)(OI)(CI)(RX)

Successfully processed 1 files; Failed processing 0 files

Running attrib on the workspace, and build dirs:

C:\Temp\my-javafx-application>attrib
AC:\Temp\my-javafx-application\.description
AC:\Temp\my-javafx-application\.gitignore
AC:\Temp\my-javafx-application\build.gradle
AC:\Temp\my-javafx-application\gradle.properties
AC:\Temp\my-javafx-application\gradlew
AC:\Temp\my-javafx-application\gradlew.bat
AC:\Temp\my-javafx-application\Jenkinsfile
AC:\Temp\my-javafx-application\packager.gradle
AC:\Temp\my-javafx-application\README.adoc
AC:\Temp\my-javafx-application\settings.gradle
AC:\Temp\my-javafx-application\spotless.gradle

C:\Temp\my-javafx-application>attrib build
  C:\Temp\my-javafx-application\build

C:\Temp\meos-dashboard>attrib build\native
  C:\Temp\my-javafx-application\build\native

/Sverre

tir. 5. okt. 2021 kl. 10:41 skrev Alan Bateman:


On 05/10/2021 08:54, Sverre Moe wrote:

With JDK 17, jpackage fails to write to the destination directory on
Windows.

It worked fine with JDK 11 (with jpackage from JDK14) and Docker.

Only happens on Windows docker. Running directly on WIndows it works

with

JDK 17.

What has changed with jpackage that it no longer can write to the
destination directory when running in Docker?
Is it a regression/bug with jpackage?


I don't know what has changed in jpackage, maybe it didn't check for
write access previously. However, the error you are seeing suggests
there may be a lower-level issue, maybe a driver issue. It would be
useful if you could print out the DACL (using cacls is okay) and the DOS
attributes.

-Alan

Re: RFR: 8280357: user.home = "?" when running with systemd DynamicUser=true

On Fri, 18 Feb 2022 16:15:18 GMT, Roger Riggs  wrote:

> I'm uncertain whether the fallback should only be done on Linux to cover the 
> `systemd` case and Docker.
> The need for a fallback seems less applicable on macOs, but since $HOME is 
> usually set to the same value, may be harmless.

I think what you have is okay because it is only used when getpwent doesn't 
return something useful. It would add complexity if you tried to limit it 
systemd or containers. I can't think of scenarios on macOS that might trigger 
the fallback, but it's not a bad fallback to have there too.

-

PR: https://git.openjdk.java.net/jdk/pull/7534

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v14]

On Thu, 17 Feb 2022 05:45:54 GMT, Stuart Marks  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fix test
>
> OK, good progress. Yes, leaving ConcurrentHashMap to another PR is fine.
> 
> Do the changes to Class.java and the enum optimal capacity test need to be 
> part of this PR? They seem unrelated. It's not clear to me that test is 
> actually testing anything useful; it's just testing the same computation a 
> couple different ways.
> 
> The test seems reasonable enough and is a good start. There are a number of 
> things that could be improved about it though.
> 
> 1) It should probably be a TestNG test. This will allow you to use a 
> DataProvider and also use TestNG assertions.
> 
> 2) There are 12 test cases here, which seems amenable to using a 
> DataProvider. You could try to make a little "combo" test that combines the 
> three classes with the four ways of creating them, but it might be difficult 
> to do that without using reflection. It might be easier to write a table with 
> 12 cases, even if there is a bit of repetition.
> 
> 3) Instead of writing reflective code to create the maps, it's probably 
> easier to use suppliers that create the maps into the desired state. Each of 
> the 12 test cases should have a lambda that does the creation. The test 
> method then invokes the supplier and makes its assertions over the resulting 
> map instance.
> 
> 4) The `fill12` method can be used in an expression if it returns its 
> argument.
> 
> 5) Create a map with 12 entries as part of the test setup, and then just use 
> it as the copy constructor argument.
> 
> Note, I'll be on vacation next week, so there will be a gap in my responses 
> during that time.

@stuart-marks please have a look again, thanks!

-

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v9]

2022-02-18 Thread Joe Darcy

On Fri, 18 Feb 2022 14:53:42 GMT, Roger Riggs  wrote:

> The Location enum does give more control over the places modifiers can occur 
> and be extended as needed. But its unfortunate there's no (simple/obvious) 
> mapping to the other concepts of the corresponding types, such as ElementType 
> or Class/Method/Constructor. I don't have a clear use case for the mapping, 
> so maybe its just a rough edge that can be smoothed out when AccessFlags gets 
> used.

A near-future iteration of this work will include functionality to map from an 
integer to a set of access flags. Since there are flags with the same mask 
position, such as volatile and bridge, some location context is needed to know 
which mapping

0x_0040 -> BRIDGE
0x_0040 -> VOLATILE

is correct and desired in context.

The Location enum could include a condensed primer on how Java programs get 
compiled into class files (constructors are just weird static methods to the 
VM!), but I didn't think that was necessary for the initial version and the 
included comment "Just stub-out constant descriptions for now." was 
meant to imply more detailed discussion would be forthcoming.

-

PR: https://git.openjdk.java.net/jdk/pull/7445

Re: RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding

On Thu, 17 Feb 2022 22:50:37 GMT, Tyler Steele  wrote:

> FileEncodingTest expects all non-Windows platforms will have 
> `Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set 
> to COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.
> 
> According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
> `Charset.defaultCharset().name()` to equal 
> `System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
> JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
> run-time value of file.encoding will be the same as the run-time value of 
> native.encoding...". So one way to resolve this is to choose the value for 
> each system from the native.encoding property.
> 
> With these changes however, my test systems continue to fail. 
> 
> - AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
> - Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> - Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
> 
> Note that the expected value is populated from native.encoding.
> 
> This implies more work to be done. It looks to me that some modification to 
> java_props_md.c may be needed to ensure that the System properties for 
> native.encoding return [canonical 
> names](http://www.iana.org/assignments/character-sets). 
> 
> ---
> 
> A tempting alternative is to set the expected value for AIX to "ISO-8859-1" 
> in the test explicitly, as was done for the Windows expected encoding prior 
> to this proposed change. The main advantage to this alternative is that it is 
> quick and easy, but the disadvantages are that it fails to test that COMPAT 
> behaves as specified in JEP-400, and the approach does not scale well if it 
> happens that other systems require other cases. I wonder if this is the 
> reason non-English locals are excluded by the test.
> 
> Proceeding with this change and the work implied by the new failures it 
> highlights goes beyond the scope of what I thought was a simple testbug. So 
> I'm opening this up for some comments before proceeding down the rabbit hole 
> of further changes. If there is generally positive support for this direction 
> I'm happy to make the modifications necessary to populate native.encoding 
> with canonical names. As I am new to OpenJDK, I am especially looking to 
> ensure that changing the value returned by native.encoding will not have 
> unintended consequences elsewhere in the project.

The purpose of this test is to check the default encoding for the environments 
known to be correct. Thus those test values are hardcoded. Replacing it with 
`System.getProperty("native.encoding")` would introduce some uncertainty 
because the test may not be run under the C locale.
As to the suggestion to canonicalize `native.encoding`, it was introduced for 
users to obtain the encoding name that used to be the value of `file.encoding` 
prior to JEP 400. So normalizing it to the canonicalized name, such as 
`ANSI_X3.4-1968` to `US-ASCII` would somewhat defy the purpose.
Now, back to the test case, the test blindly assumes that C locale's default 
code set is `US-ASCII` which is not correct (as in this issue), it only 
requires Portable Character Set, which US-ASCII/ISO-8859-1/UTF-8 all suffice. I 
would change the test to check if the platform is AIX, then check the charset 
for COMPAT to ISO-8859-1.

-

PR: https://git.openjdk.java.net/jdk/pull/7525

Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v3]

On Fri, 18 Feb 2022 17:21:48 GMT, Olga Mikhaltsova  
wrote:

>> This fix made equal processing of strings such as ""C:\\Program 
>> Files\\Git\\"" before and after JDK-8250568.
>> 
>> For example, it's needed to execute the following command on Windows:
>> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
>> it's equal to:
>> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", 
>> ""C:\\Program Files\\Git\\"", "Test").start();`
>> 
>> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
>> unquoted due to the condition added in JDK-8250568.
>> 
>> private static String unQuote(String str) {
>> .. 
>>if (str.endsWith("\\"")) {
>> return str;// not properly quoted, treat as unquoted
>> }
>> ..
>> }
>> 
>> 
>> that leads to the additional surrounding by quotes in 
>> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true 
>> due to the space inside the string argument.
>> As a result the native function CreateProcessW 
>> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
>> quoted argument: 
>> 
>> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = true)
>> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
>> Test
>> (jdk.lang.Process.allowAmbiguousCommands = false)
>> 
>> 
>> Obviously, a string ending with `"\\""` must not be started with `"""` to 
>> treat as unquoted overwise it’s should be treated as properly quoted.
>
> Olga Mikhaltsova has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add a new line to the end of test file for JDK-8282008

ProcessBuilder handles the quoting of arguments with spaces.
In your QuotedArguments.java, just remove the quotes from the first argument to 
CheckCase:

```CheckCase[] cases = {
new CheckCase("C:\\Program Files\\Git\", "C:\\Program 
Files\\Git\", "true"),
new CheckCase("C:\\Program Files\\Git\", "C:\\Program 
Files\\Git\", "false")
};

That worked for me using openjdk version "17.0.2".
Problems with knowing what and if to quote go back a long time.
I'm working on a new [JEP](https://bugs.openjdk.java.net/browse/JDK-8263697) to 
handle more of the cases without application intervention.

-

PR: https://git.openjdk.java.net/jdk/pull/7504

Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v3]

2022-02-18 Thread Olga Mikhaltsova

On Fri, 18 Feb 2022 17:21:48 GMT, Olga Mikhaltsova  
wrote:

>> This fix made equal processing of strings such as ""C:\\Program 
>> Files\\Git\\"" before and after JDK-8250568.
>> 
>> For example, it's needed to execute the following command on Windows:
>> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
>> it's equal to:
>> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", 
>> ""C:\\Program Files\\Git\\"", "Test").start();`
>> 
>> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
>> unquoted due to the condition added in JDK-8250568.
>> 
>> private static String unQuote(String str) {
>> .. 
>>if (str.endsWith("\\"")) {
>> return str;// not properly quoted, treat as unquoted
>> }
>> ..
>> }
>> 
>> 
>> that leads to the additional surrounding by quotes in 
>> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true 
>> due to the space inside the string argument.
>> As a result the native function CreateProcessW 
>> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
>> quoted argument: 
>> 
>> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = true)
>> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
>> Test
>> (jdk.lang.Process.allowAmbiguousCommands = false)
>> 
>> 
>> Obviously, a string ending with `"\\""` must not be started with `"""` to 
>> treat as unquoted overwise it’s should be treated as properly quoted.
>
> Olga Mikhaltsova has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add a new line to the end of test file for JDK-8282008

Roger, thanks for your comments! But in this case how it is possible to present 
a path ending with '\' and including a space inside?

-

PR: https://git.openjdk.java.net/jdk/pull/7504

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v4]

On Fri, 18 Feb 2022 17:15:17 GMT, Lance Andersen  wrote:

> If you feel there is still something lacking for documentation, I can 
> certainly make another pass clarify/add it, but I tried to cover the steps 
> (but I also understand what might be obvious to me might not be as obvious as 
> I thought).

Yes, I think clear instructions are important to have for tests like this. It 
doesn't need much but what you know is scattered across a method and a comment 
on a byte array, just not clear/easy.

-

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v3]

2022-02-18 Thread Olga Mikhaltsova

> This fix made equal processing of strings such as ""C:\\Program 
> Files\\Git\\"" before and after JDK-8250568.
> 
> For example, it's needed to execute the following command on Windows:
> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
> it's equal to:
> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", 
> ""C:\\Program Files\\Git\\"", "Test").start();`
> 
> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
> unquoted due to the condition added in JDK-8250568.
> 
> private static String unQuote(String str) {
> .. 
>if (str.endsWith("\\"")) {
> return str;// not properly quoted, treat as unquoted
> }
> ..
> }
> 
> 
> that leads to the additional surrounding by quotes in 
> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true due 
> to the space inside the string argument.
> As a result the native function CreateProcessW 
> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
> quoted argument: 
> 
> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
> (jdk.lang.Process.allowAmbiguousCommands = true)
> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
> Test
> (jdk.lang.Process.allowAmbiguousCommands = false)
> 
> 
> Obviously, a string ending with `"\\""` must not be started with `"""` to 
> treat as unquoted overwise it’s should be treated as properly quoted.

Olga Mikhaltsova has updated the pull request incrementally with one additional 
commit since the last revision:

  Add a new line to the end of test file for JDK-8282008

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7504/files
  - new: https://git.openjdk.java.net/jdk/pull/7504/files/6a2c82f9..0eceecac

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7504=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7504=01-02

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7504.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7504/head:pull/7504

PR: https://git.openjdk.java.net/jdk/pull/7504

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v4]

On Fri, 18 Feb 2022 17:05:53 GMT, Alan Bateman  wrote:

> > > The updates changes to ZipFile/JarFile look okay. I don't have time to 
> > > study the test too closely right now but it will need to include 
> > > instructions on how to re-create the signed JAR that is stored in the 
> > > byte array.
> > 
> > 
> > Those instructions are already in the comments for the constant 
> > `SIGNED_VALID_ENTRY_NAME`
> 
> That's the keytool command to sign the JAR. What I meant is the complete 
> steps to create the JAR file, sign it, and then create the byte array.

The `createByteArray` method which is at the bottom of the test class documents 
how the byte array  can be made from a jar.

The signed jar is created using the steps defined in `SIGNED_VALID_ENTRY_NAME` 
from the jar   derived from `VALID_ENTRY_NAME`

If you feel there is still something lacking for documentation, I can certainly 
make another pass  clarify/add it, but I tried to cover the steps (but I also 
understand what might be obvious to me might not be as obvious as I thought).

-

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v4]

On Fri, 18 Feb 2022 16:25:30 GMT, Lance Andersen  wrote:

> > The updates changes to ZipFile/JarFile look okay. I don't have time to 
> > study the test too closely right now but it will need to include 
> > instructions on how to re-create the signed JAR that is stored in the byte 
> > array.
> 
> Those instructions are already in the comments for the constant 
> `SIGNED_VALID_ENTRY_NAME`

That's the keytool command to sign the JAR. What I meant is the complete steps 
to create the JAR file, sign it, and then create the byte array.

-

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v2]

On Fri, 18 Feb 2022 16:07:29 GMT, Olga Mikhaltsova  
wrote:

>> This fix made equal processing of strings such as ""C:\\Program 
>> Files\\Git\\"" before and after JDK-8250568.
>> 
>> For example, it's needed to execute the following command on Windows:
>> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
>> it's equal to:
>> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", 
>> ""C:\\Program Files\\Git\\"", "Test").start();`
>> 
>> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
>> unquoted due to the condition added in JDK-8250568.
>> 
>> private static String unQuote(String str) {
>> .. 
>>if (str.endsWith("\\"")) {
>> return str;// not properly quoted, treat as unquoted
>> }
>> ..
>> }
>> 
>> 
>> that leads to the additional surrounding by quotes in 
>> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true 
>> due to the space inside the string argument.
>> As a result the native function CreateProcessW 
>> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
>> quoted argument: 
>> 
>> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = true)
>> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
>> Test
>> (jdk.lang.Process.allowAmbiguousCommands = false)
>> 
>> 
>> Obviously, a string ending with `"\\""` must not be started with `"""` to 
>> treat as unquoted overwise it’s should be treated as properly quoted.
>
> Olga Mikhaltsova has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add test for JDK-8282008

@omikhaltsova Please take another look at the comment above.  The fix 
incorrectly allows a final double-quote to be escaped, resulting in unbalanced 
quotes and possibly allowing an argument to be joined with the next.
The recommendation is for the application to NOT add quotes to arguments and 
allow ProcessBuilder to do the necessary quoting.

-

PR: https://git.openjdk.java.net/jdk/pull/7504

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v4]

On Fri, 18 Feb 2022 12:09:53 GMT, Alan Bateman  wrote:

> The updates changes to ZipFile/JarFile look okay. I don't have time to study 
> the test too closely right now but it will need to include instructions on 
> how to re-create the signed JAR that is stored in the byte array.

Those instructions are already in the comments for the constant 
`SIGNED_VALID_ENTRY_NAME`

-

PR: https://git.openjdk.java.net/jdk/pull/7348

Re: RFR: 8280357: user.home = "?" when running with systemd DynamicUser=true

On Fri, 18 Feb 2022 15:29:34 GMT, Roger Riggs  wrote:

> In some Linux configurations, the Linux home directory provided by getpwent 
> is not usable.
> The value of the system property `user.home` should fallback to the value of 
> $HOME 
> if getpwent.user_home is null or less that 2 characters long. "/" is not a 
> valid home directory name.
> 
> If $HOME is undefined or empty, the value of the getpwent.user_home is 
> retained.
> 
> There are more details in the Jira issue: 
> https://bugs.openjdk.java.net/browse/JDK-8280357
> 
> The fix has been tested manually on Ubuntu 20.0.4 using the suggested systemd 
> command line and variations.

I'm uncertain whether the fallback should only be done on Linux to cover the 
`systemd` case and Docker.
The need for a fallback seems less applicable on macOs, but since $HOME is 
usually set to the same value, may be harmless.

-

PR: https://git.openjdk.java.net/jdk/pull/7534

Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v2]

2022-02-18 Thread Olga Mikhaltsova

> This fix made equal processing of strings such as ""C:\\Program 
> Files\\Git\\"" before and after JDK-8250568.
> 
> For example, it's needed to execute the following command on Windows:
> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
> it's equal to:
> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", 
> ""C:\\Program Files\\Git\\"", "Test").start();`
> 
> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
> unquoted due to the condition added in JDK-8250568.
> 
> private static String unQuote(String str) {
> .. 
>if (str.endsWith("\\"")) {
> return str;// not properly quoted, treat as unquoted
> }
> ..
> }
> 
> 
> that leads to the additional surrounding by quotes in 
> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true due 
> to the space inside the string argument.
> As a result the native function CreateProcessW 
> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
> quoted argument: 
> 
> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
> (jdk.lang.Process.allowAmbiguousCommands = true)
> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
> Test
> (jdk.lang.Process.allowAmbiguousCommands = false)
> 
> 
> Obviously, a string ending with `"\\""` must not be started with `"""` to 
> treat as unquoted overwise it’s should be treated as properly quoted.

Olga Mikhaltsova has updated the pull request incrementally with one additional 
commit since the last revision:

  Add test for JDK-8282008

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7504/files
  - new: https://git.openjdk.java.net/jdk/pull/7504/files/721d4023..6a2c82f9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7504=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7504=00-01

  Stats: 88 lines in 1 file changed: 88 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7504.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7504/head:pull/7504

PR: https://git.openjdk.java.net/jdk/pull/7504

Re: RFR: 8281146: Replace StringCoding.hasNegatives with countPositives [v4]

2022-02-18 Thread Claes Redestad

> I'm requesting comments and, hopefully, some help with this patch to replace 
> `StringCoding.hasNegatives` with `countPositives`. The new method does a very 
> similar pass, but alters the intrinsic to return the number of leading bytes 
> in the `byte[]` range which only has positive bytes. This allows for dealing 
> much more efficiently with those `byte[]`s that has a ASCII prefix, with no 
> measurable cost on ASCII-only or latin1/UTF16-mostly input.
> 
> Microbenchmark results: 
> https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904
> 
> - Only implemented on x86 for now, but I want to verify that implementations 
> of `countPositives` can be implemented with similar efficiency on all 
> platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) 
> before moving ahead. This pretty much means holding up this until it's 
> implemented on all platforms, which can either contributed to this PR or as 
> dependent follow-ups.
> 
> - An alternative to holding up until all platforms are on board is to allow 
> the implementation of `StringCoding.hasNegatives` and `countPositives` to be 
> implemented so that the non-intrinsified method calls into the intrinsified. 
> This requires structuring the implementations differently based on which 
> intrinsic - if any - is actually implemented. One way to do this could be to 
> mimic how `java.nio` handles unaligned accesses and expose which intrinsic is 
> available via `Unsafe` into a `static final` field.
> 
> - There are a few minor regressions (~5%) in the x86 implementation on 
> `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, 
> for example `encode-/decodeShortMixed` even see a minor improvement, which 
> makes me consider those corner case regressions with little real world 
> implications (if you have latin1 Strings, you're likely to also have 
> ASCII-only strings in your mix).

Claes Redestad has updated the pull request incrementally with one additional 
commit since the last revision:

  Switch aarch64 intrinsic to a variant of countPositives returning len or zero 
as a first step.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7231/files
  - new: https://git.openjdk.java.net/jdk/pull/7231/files/531139a1..a5e28b32

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7231=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7231=02-03

  Stats: 59 lines in 8 files changed: 7 ins; 14 del; 38 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7231.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7231/head:pull/7231

PR: https://git.openjdk.java.net/jdk/pull/7231

Re: RFR: 8280357: user.home = "?" when running with systemd DynamicUser=true

On Fri, 18 Feb 2022 15:29:34 GMT, Roger Riggs  wrote:

> In some Linux configurations, the Linux home directory provided by getpwent 
> is not usable.
> The value of the system property `user.home` should fallback to the value of 
> $HOME 
> if getpwent.user_home is null or less that 2 characters long. "/" is not a 
> valid home directory name.
> 
> If $HOME is undefined or empty, the value of the getpwent.user_home is 
> retained.
> 
> There are more details in the Jira issue: 
> https://bugs.openjdk.java.net/browse/JDK-8280357
> 
> The fix has been tested manually on Ubuntu 20.0.4 using the suggested systemd 
> command line and variations.

There is an argument that the JDK should read $HOME first but changing it after 
20+ years could be risky, probably difficult to get a list of 
configurations/environments where the two might yield different locations. So I 
think the approach is reasonable. Even if "/" were a configured as the home 
directory in the user entry then $HOME would need to agree so that I think the 
<2 check is okay too.

-

PR: https://git.openjdk.java.net/jdk/pull/7534

RFR: 8280357: user.home = "?" when running with systemd DynamicUser=true

In some Linux configurations, the Linux home directory provided by getpwent is 
not usable.
The value of the system property `user.home` should fallback to the value of 
$HOME 
if getpwent.user_home is null or less that 2 characters long. "/" is not a 
valid home directory name.

If $HOME is undefined or empty, the value of the getpwent.user_home is retained.

There are more details in the Jira issue: 
https://bugs.openjdk.java.net/browse/JDK-8280357

The fix has been tested manually on Ubuntu 20.0.4 using the suggested systemd 
command line and variations.

-

Commit messages:
 - Fallback to $HOME, if defined, if the os supplied user_home is
 - Fallback to $HOME if the home directory is "/" or not available
 - 8280357: user.home = "?" when running with systemd DynamicUser=true

Changes: https://git.openjdk.java.net/jdk/pull/7534/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7534=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8280357
  Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7534.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7534/head:pull/7534

PR: https://git.openjdk.java.net/jdk/pull/7534

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v19]

> 8281631: HashMap copy constructor and putAll can over-allocate table

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

 - migrate to junit
 - change threshold

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7431/files
  - new: https://git.openjdk.java.net/jdk/pull/7431/files/cdfb03bb..aa599698

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=18
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=17-18

  Stats: 64 lines in 2 files changed: 17 ins; 17 del; 30 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7431.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v18]

On Fri, 18 Feb 2022 11:20:12 GMT, XenoAmess  wrote:

> well seems jtreg cannot invoke Junit4 's parameterized test.

Nope, it can! :)

-

PR: https://git.openjdk.java.net/jdk/pull/7431

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v9]

On Fri, 18 Feb 2022 03:38:36 GMT, Joe Darcy  wrote:

>> This is an early review of changes to better model JVM access flags, that is 
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>> 
>> Language level modifiers and JVM level access flags are closely related, but 
>> distinct. There are concepts that overlap in the two domains (public, 
>> private, etc.), others that only have a language-level modifier (sealed), 
>> and still others that only have an access flag (synthetic).
>> 
>> The existing java.lang.reflect.Modifier class is inadequate to model these 
>> subtleties. For example, the bit positions used by access flags on different 
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
>> methods. Just having a raw integer does not provide sufficient context to 
>> decode the corresponding language-level string. Methods like 
>> Modifier.methodModifiers() were introduced to cope with this situation.
>> 
>> With additional modifiers and flags on the horizon with projects like 
>> Valhalla, addressing the existent modeling deficiency now ahead of time is 
>> reasonable before further strain is introduced.
>> 
>> This PR in its current form is meant to give the overall shape of the API. 
>> It is missing implementations to map from, say, method modifiers to access 
>> flags, taking into account overlaps in bit positions.
>> 
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
>> once the API is further along.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Switch to location enum.

The Location enum does give more control over the places modifiers can occur 
and be extended as needed.
But its unfortunate there's no (simple/obvious) mapping to the other concepts 
of the corresponding types, such as ElementType or Class/Method/Constructor.  
I don't have a clear use case for the mapping, so maybe its just a rough edge 
that can be smoothed out when AccessFlags gets used.

-

PR: https://git.openjdk.java.net/jdk/pull/7445

Re: Unused static constant MathContext.DEFAULT_DIGITS

2022-02-18 Thread David Holmes


On 18/02/2022 7:55 pm, Andrey Turbanov wrote:

Hello.
I noticed unused field java.math.MathContext#DEFAULT_DIGITS
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/math/MathContext.java#L61

As I can see, it was already unused in the initial OpenJDK source.
Does VM use this field somehow? Or used to use?


The VM doesn't use it. Looks like dead code.

Cheers,
David


Andrey Turbanov

Re: JDK-17: Wndows jpackage destination directory not writable

2022-02-18 Thread Sverre Moe

I executed our JDK11 docker image, which works fine with JDK11 and JDK14
(for jpackage support).
Then I installed the JDK17 MSI package, changed JAVA_HOME and PATH.
Building our application now with JDK17 it still cannot write to the
"build/native" jpackage output directory.

Leads me to conclude something has changed in jpackage between JDK14 and
JDK17.
I modified my gradle configuration, to use jpackage from JDK14 when
packaging my JDK17 built application.
It works using JDK14 jpackage to package our JDK17 application.

Using the JDK17 jpackage directly in Windows works fine. It is only in the
Docker container that it does not work.

/Sverre



tir. 5. okt. 2021 kl. 11:55 skrev Sverre Moe :

> I ran cacls after the failed jpackage.
>
> C:\temp\my-javafx-application>cacls build
> C:\temp\my-javafx-application\build F
>  CREATOR OWNER:(OI)(CI)(IO)F
>  R
>  CREATOR GROUP:(OI)(CI)(IO)R
>  Everyone:(OI)(CI)R
>
> C:\temp\my-javafx-application>cacls build\native
> C:\temp\my-javafx-application\build\native F
> CREATOR OWNER:(OI)(CI)(IO)F
> R
> CREATOR GROUP:(OI)(CI)(IO)R
> Everyone:(OI)(CI)R
>
>
> As cacls was deprecated it suggested to use icacls instead:
>
> C:\temp\my-javafx-application>icacls build
> build S-1-5-21-406077803-2019195645-689573112-1003:(I)(F)
>   CREATOR OWNER:(I)(OI)(CI)(IO)(F)
>   S-1-5-21-406077803-2019195645-689573112-513:(I)(RX)
>   CREATOR GROUP:(I)(OI)(CI)(IO)(RX)
>   Everyone:(I)(OI)(CI)(RX)
>
> Successfully processed 1 files; Failed processing 0 files
>
> C:\temp\my-javafx-application>icacls build\native
> build\native S-1-5-83-1-1537791174-1084404783-2478187907-2577323605:(I)(F)
>  CREATOR OWNER:(I)(OI)(CI)(IO)(F)
>  S-1-5-83-0:(I)(RX)
>  CREATOR GROUP:(I)(OI)(CI)(IO)(RX)
>  Everyone:(I)(OI)(CI)(RX)
>
> Successfully processed 1 files; Failed processing 0 files
>
> Running attrib on the workspace, and build dirs:
>
> C:\Temp\my-javafx-application>attrib
> AC:\Temp\my-javafx-application\.description
> AC:\Temp\my-javafx-application\.gitignore
> AC:\Temp\my-javafx-application\build.gradle
> AC:\Temp\my-javafx-application\gradle.properties
> AC:\Temp\my-javafx-application\gradlew
> AC:\Temp\my-javafx-application\gradlew.bat
> AC:\Temp\my-javafx-application\Jenkinsfile
> AC:\Temp\my-javafx-application\packager.gradle
> AC:\Temp\my-javafx-application\README.adoc
> AC:\Temp\my-javafx-application\settings.gradle
> AC:\Temp\my-javafx-application\spotless.gradle
>
> C:\Temp\my-javafx-application>attrib build
>  C:\Temp\my-javafx-application\build
>
> C:\Temp\meos-dashboard>attrib build\native
>  C:\Temp\my-javafx-application\build\native
>
> /Sverre
>
> tir. 5. okt. 2021 kl. 10:41 skrev Alan Bateman :
>
>> On 05/10/2021 08:54, Sverre Moe wrote:
>> > With JDK 17, jpackage fails to write to the destination directory on
>> > Windows.
>> >
>> > It worked fine with JDK 11 (with jpackage from JDK14) and Docker.
>> >
>> > Only happens on Windows docker. Running directly on WIndows it works
>> with
>> > JDK 17.
>> >
>> > What has changed with jpackage that it no longer can write to the
>> > destination directory when running in Docker?
>> > Is it a regression/bug with jpackage?
>> >
>> I don't know what has changed in jpackage, maybe it didn't check for
>> write access previously. However, the error you are seeing suggests
>> there may be a lower-level issue, maybe a driver issue. It would be
>> useful if you could print out the DACL (using cacls is okay) and the DOS
>> attributes.
>>
>> -Alan
>>
>

Re: RFR: 8280409: JarFile::getInputStream can fail with NPE accessing ze.getName() [v4]

On Thu, 17 Feb 2022 19:00:47 GMT, Lance Andersen  wrote:

>> Hi all,
>> 
>> Please review the attached patch to address
>> 
>> - That JarFile::getInputStream did not check for a null ZipEntry passed as a 
>> parameter
>> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an 
>> unexpected exception occurs
>> 
>> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar 
>> test runs
>> 
>> Best
>> Lance
>
> Lance Andersen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Return null when ZipEntry::getName is null

The updates changes to ZipFile/JarFile look okay. I don't have time to study 
the test too closely right now but it will need to include instructions on how 
to re-create the signed JAR that is stored in the byte array.

-

PR: https://git.openjdk.java.net/jdk/pull/7348

Accessing the BCI of Throwable's StackTraceElement

2022-02-18 Thread Eirik Bjørsnøs

Hi,

Currently, StackWalker.StackFrame::getByteCodeIndex provides a way to get
the BCI of stack frames during a stack walk.

Similarly, it might be useful to get the BCI when inspecting a Throwable's
StackTraceElements. This does however not seem possible, given that
StackTraceElement currently does not expose the BCI. (It exposes the
lineNumber, but that's not useful for my current use case).

On another note, it seems like calling Throwable.getStackTrace() would be a
wasteful way to just find the BCI of the top stack frame. Would be really
nice if one could do a StackWalk with a Throwable as input, which I imagine
should work a lot more efficiently.

(My use case here is to attribute any exception to a BCI in the same method
such that I don't need to instrument code to track the BCI myself).

So I guess I have two questions:

1: Would it be reasonable to add a getByteCodeIndex method to
StackTraceElement? Or perhaps even make StackTraceElement implement
StackWalker.StackFrame?

2: Could the StackWalker API be suitable for walking not just the stack of
current threads, but also the stacks of Throwables? Was this discussed
during the development of StackWalker for Java 9?

Cheers,
Eirik.

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v18]