Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v6]

[Yi Yang]

> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
> by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
> the whole JDK codebase.

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

  more replacement 2

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4507/files
  - new: https://git.openjdk.java.net/jdk/pull/4507/files/c8b2106e..3a8875ec

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

  Stats: 32 lines in 2 files changed: 1 ins; 20 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4507.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4507/head:pull/4507

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

Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v5]

[Yi Yang]

> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
> by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
> the whole JDK codebase.

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

  more replacements

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4507/files
  - new: https://git.openjdk.java.net/jdk/pull/4507/files/154989a0..c8b2106e

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

  Stats: 59 lines in 8 files changed: 11 ins; 37 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4507.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4507/head:pull/4507

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

Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v4]

[Yi Yang]

> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
> by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
> the whole JDK codebase.

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

  format

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4507/files
  - new: https://git.openjdk.java.net/jdk/pull/4507/files/c1dd2f76..154989a0

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

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

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

Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v3]

[Yi Yang]

> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
> by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
> the whole JDK codebase.

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

  new Preconditions.IOOBE_FORMATTER

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4507/files
  - new: https://git.openjdk.java.net/jdk/pull/4507/files/ec0a4d44..c1dd2f76

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

  Stats: 129 lines in 13 files changed: 43 ins; 40 del; 46 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4507.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4507/head:pull/4507

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

Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v2]

[Yi Yang]

On Fri, 18 Jun 2021 18:03:44 GMT, Paul Sandoz  wrote:

>> Yi Yang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   restore IndexOfOufBoundsException; split exception line
>
> src/java.base/share/classes/java/util/Base64.java line 935:
> 
>> 933: throw new IOException("Stream is closed");
>> 934: Preconditions.checkFromIndexSize(len, off, b.length,
>> 935: 
>> Preconditions.outOfBoundsExceptionFormatter(ArrayIndexOutOfBoundsException::new));
> 
> `outOfBoundsExceptionFormatter` will allocate. Store the result of 
> `Preconditions.outOfBoundsExceptionFormatter(ArrayIndexOutOfBoundsException::new))`
>  in a public static final on `Preconditions`, and replace the method ref with 
> a inner class (thereby making it usable earlier at VM startup.

Thanks for the clarification! Fixed.

This incremental change does many stuff:
- Create inner classes and public static final fields within Preconditions
- Use Preconditions.check* in j.l.String
- Use Preconditions.*IOOBE_FORMATTER in java.util.zip.* classes
- Use Preconditions.*IOOBE_FORMATTER in java.util.Base64
- Use Preconditions.*IOOBE_FORMATTER in X-VarHandle.java.template and 
X-VarHandleByteArrayView.java.template
- Use Preconditions.*IOOBE_FORMATTER in sun.security.provider.DigestBase
- Use Preconditions.*IOOBE_FORMATTER in sun.security.util.ArrayUtil

-

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

Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo

[Alan Bateman]

On 18/06/2021 18:30, Brian Burkhalter wrote:


On Jun 18, 2021, at 6:36 AM, Alan Bateman > wrote:


Adding an override of transferTo may require new tests. @bplb Do you 
if we have good tests for all the scenarios where input stream 
returned by Channels.newInputStream is the source?


There are not a lot of tests for the `InputStream` returned by 
`Channels.newInputStream`:


 `java/nio/channels/Channels`:

`Basic.java` - reading from a wrapped `FileChannel`
`ReadByte.java` - trivial
`Basic2.java` - reading at random offsets from a wrapped 
`AsynchronousSocketChannel`

`ReadOffset.java` - trivial

Elsewhere:

`sun/nio/ch/TempBuffer.java`
`jdk/nio/zipfs/ZipFSTester.java`


I don’t see that `transferTo()` on such a stream is tested anywhere.


InputStream/TransferTo.java tests the default implementation. Maybe we 
re-structured this to use a data provider so that the tests exercise 
both the default implementation and the various overrides. Alternatively 
a new test that exercises the various overrides in different scenarios. 
In any case, I think the feedback for PR 4263 is that it is essentially 
several implementations of transferTo and each of those need to have 
test coverage.


-Alan

Re: RFR: 8268974: jpackage fails to handle --dest option containing "bin" folder

[Alan Bateman]

On Fri, 18 Jun 2021 22:46:24 GMT, Alexey Semenyuk  wrote:

> GetApplicationHomeFromDll() fails if the path to libjli.so contains "bin" 
> component (/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so). TruncatePath() 
> looks for "/bin/" first in "/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so" 
> string and then it looks for "/lib/". But this is wrong order as it should 
> look for "/lib/" first. I.e. TruncatePath() should look for "/bin/" and then 
> for "/lib/" if called from GetApplicationHome() and for "/lib/" first and 
> then for "/bin/" if called from GetApplicationHomeFromDll().

Is it possible to add a test for this that is completely independent of 
jpackage? I think there are a few existing tests that copy the run-time image 
to a new location for testing purposes.

We may need to rename the JBS description to make it clearer what this issue is 
about.

A minor nit is that "pathisso" will be confusing to anyone looking at this 
code, maybe find a better name or put a comment in TruncatePath to explain it. 
I assume the comments at the findLastPathComponent use site will also need to 
be clarified.

-

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