Re: RFR: 8261670: Add javadoc for the XML processing limits [v2]

[Joe Wang]

> Add the documentation for XML processing limits to module summary. The limits 
> were previously documented in Java tutorial and guide.

Joe Wang has updated the pull request with a new target base due to a merge or 
a rebase. The pull request now contains two commits:

 - Merge branch 'master' into JDK-8261670
 - 8261670: Add javadoc for the XML processing limits

-

Changes: https://git.openjdk.java.net/jdk/pull/2732/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2732=01
  Stats: 87 lines in 1 file changed: 81 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2732.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2732/head:pull/2732

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v4]

[Paul Sandoz]

On Fri, 26 Feb 2021 02:38:00 GMT, Jie Fu  wrote:

>> Hi all,
>> 
>> Vector API fails to work when:
>>  - case 1: MaxVectorSize is set to <=8, or
>>  - case 2: C2 is disabled
>> 
>> The reason is that {max/preferred} VectorShape initialization fails in both 
>> cases.
>> And the root cause is that VectorSupport_GetMaxLaneCount [1] returns 
>> unreasonable values (0 for case 1 and -1 for case 2).
>> 
>> Vector API should not depend on C2 to run.
>> It should work even there is no JIT compiler since it's a Java-level api.
>> So let's fix it.
>> 
>> Testing:
>>   - jdk/incubator/vector with -XX:MaxVectorSize=default/8 on Linux/x64
>> 
>> Thanks.
>> Best regards,
>> Jie
>> 
>> [1] 
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/vectorSupport.cpp#L364
>
> Jie Fu has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Remove -XX:TieredStopAtLevel=3

Marked as reviewed by psandoz (Reviewer).

-

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v4]

[Jie Fu]

> Hi all,
> 
> Vector API fails to work when:
>  - case 1: MaxVectorSize is set to <=8, or
>  - case 2: C2 is disabled
> 
> The reason is that {max/preferred} VectorShape initialization fails in both 
> cases.
> And the root cause is that VectorSupport_GetMaxLaneCount [1] returns 
> unreasonable values (0 for case 1 and -1 for case 2).
> 
> Vector API should not depend on C2 to run.
> It should work even there is no JIT compiler since it's a Java-level api.
> So let's fix it.
> 
> Testing:
>   - jdk/incubator/vector with -XX:MaxVectorSize=default/8 on Linux/x64
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/vectorSupport.cpp#L364

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

  Remove -XX:TieredStopAtLevel=3

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2722/files
  - new: https://git.openjdk.java.net/jdk/pull/2722/files/aa475b0a..bbe6150c

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

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

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v4]

[Jie Fu]

On Fri, 26 Feb 2021 02:28:55 GMT, Paul Sandoz  wrote:

> In that case I think we can remove the execution with 
> `-XX:TieredStopAtLevel=x`.

Fixed. Thanks.

-

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v3]

[Paul Sandoz]

On Fri, 26 Feb 2021 02:16:02 GMT, Jie Fu  wrote:

>> Thanks, was the test `VectorShapeInitTest` passing prior to the fix of the 
>> numerator?
>> Perhaps we should be testing more directly on 
>> `VectorShape.S_Max_BIT.vectorBitSize()` and `VectorShape.preferredShape` ? 
>> Also, perhaps we can run with C2 disabled, with `-XX:TieredStopAtLevel=3` ?
>
>> Thanks, was the test `VectorShapeInitTest` passing prior to the fix of the 
>> numerator?
> Yes. It also passed with min_lane_count = 64 / type2aelembytes(bt).
> 
>> Perhaps we should be testing more directly on 
>> `VectorShape.S_Max_BIT.vectorBitSize()` and `VectorShape.preferredShape` ?
> Ok.
> But it that case, I'd like to just re-use 
> jdk/incubator/vector/PreferredSpeciesTest.java.
> 
>> Also, perhaps we can run with C2 disabled, with `-XX:TieredStopAtLevel=3` ?
> Fine.
> Although this bug wouldn't be triggered with -XX:TieredStopAtLevel=x, it can 
> help test  with C1.
> 
> Patch had been updated.
> Any comments?
> Thanks.

Reusing PreferredSpeciesTest is a good idea.

I now realize that C2 needs to be compiled out to trigger some other cases. In 
that case I think we can remove the execution with `-XX:TieredStopAtLevel=x`. 
The test will get executed with various HotSpot configurations by the test 
infrastructure, eventually...

Looks good.

-

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v3]

[Jie Fu]

> Hi all,
> 
> Vector API fails to work when:
>  - case 1: MaxVectorSize is set to <=8, or
>  - case 2: C2 is disabled
> 
> The reason is that {max/preferred} VectorShape initialization fails in both 
> cases.
> And the root cause is that VectorSupport_GetMaxLaneCount [1] returns 
> unreasonable values (0 for case 1 and -1 for case 2).
> 
> Vector API should not depend on C2 to run.
> It should work even there is no JIT compiler since it's a Java-level api.
> So let's fix it.
> 
> Testing:
>   - jdk/incubator/vector with -XX:MaxVectorSize=default/8 on Linux/x64
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/vectorSupport.cpp#L364

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

  Update the jtreg test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2722/files
  - new: https://git.openjdk.java.net/jdk/pull/2722/files/724b36d4..aa475b0a

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

  Stats: 59 lines in 2 files changed: 10 ins; 48 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2722.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2722/head:pull/2722

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v3]

[Jie Fu]

On Fri, 26 Feb 2021 00:30:45 GMT, Paul Sandoz  wrote:

> Thanks, was the test `VectorShapeInitTest` passing prior to the fix of the 
> numerator?
Yes. It also passed with min_lane_count = 64 / type2aelembytes(bt).

> Perhaps we should be testing more directly on 
> `VectorShape.S_Max_BIT.vectorBitSize()` and `VectorShape.preferredShape` ?
Ok.
But it that case, I'd like to just re-use 
jdk/incubator/vector/PreferredSpeciesTest.java.

> Also, perhaps we can run with C2 disabled, with `-XX:TieredStopAtLevel=3` ?
Fine.
Although this bug wouldn't be triggered with -XX:TieredStopAtLevel=x, it can 
help test  with C1.

Patch had been updated.
Any comments?
Thanks.

-

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

Re: ZipInputStream#readAllBytes should clarify it doesn't read the whole stream?

[Jaikiran Pai]

I just created https://bugs.openjdk.java.net/browse/JDK-8262435 to track 
this.


-Jaikiran

On 25/02/21 9:05 pm, Jaikiran Pai wrote:


Thank you Lance and Alan. I do have access to JBS, I'll file one 
tomorrow with the details.


-Jaikiran

On 25/02/21 9:04 pm, Lance Andersen wrote:

Hi Jaikiran,

Yes I believe this makes sense.

It would also require a CSR.

I can login a bug if you do not have access to do so.

Best
Lance

On Feb 24, 2021, at 10:56 PM, Jaikiran Pai > wrote:


The javadoc of InputStream#readAllBytes() states[1] that it reads 
all the remaining bytes of the stream. The 
java.util.zip.ZipInputStream doesn't override this method and thus 
"inherits" this javadoc. The implementation of 
InputStream#readAllBytes() ultimately ends up calling 
ZipInputStream#read()[2], so the implementation correctly reads only 
till the end of the current ZipEntry and not the entire 
ZipInputStream. However, because the javadoc gets inherited, reading 
any code like the following doesn't make it clear that it's only 
reading till the end of the current entry:


zis = ... // ZipInputStream
while ((e = zis.getNextEntry()) != null) {
    String name = e.getName();
    zis.readAllBytes(); // gives an impression that all bytes of the 
stream are read

    ...
}

Should the ZipInputStream override the readAllBytes(), just so as to 
add a very specific javadoc to this method which explains that it 
reads only till the end of current entry and other related 
semantics? Perhaps the same should be done for 
ZipInputStream#readNBytes(...)?



[1] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/io/InputStream.html#readAllBytes() 

[2] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/zip/ZipInputStream.html#read(byte%5B%5D,int,int) 



-Jaikiran






Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: JDK-8262433: doclint: reference error in module jdk.incubator.foreign

[Brian Burkhalter]

On Fri, 26 Feb 2021 00:41:18 GMT, Jonathan Gibbons  wrote:

> Please review a simple fix to ensure that a link reference is resolved and 
> displayed correctly.

Marked as reviewed by bpb (Reviewer).

-

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

Integrated: JDK-8262433: doclint: reference error in module jdk.incubator.foreign

[Jonathan Gibbons]

On Fri, 26 Feb 2021 00:41:18 GMT, Jonathan Gibbons  wrote:

> Please review a simple fix to ensure that a link reference is resolved and 
> displayed correctly.

This pull request has now been integrated.

Changeset: fce57656
Author:Jonathan Gibbons 
URL:   https://git.openjdk.java.net/jdk/commit/fce57656
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8262433: doclint: reference error in module jdk.incubator.foreign

Reviewed-by: bpb, lancea

-

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

Re: RFR: JDK-8262433: doclint: reference error in module jdk.incubator.foreign

[Lance Andersen]

On Fri, 26 Feb 2021 00:41:18 GMT, Jonathan Gibbons  wrote:

> Please review a simple fix to ensure that a link reference is resolved and 
> displayed correctly.

Marked as reviewed by lancea (Reviewer).

-

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

RFR: JDK-8262433: doclint: reference error in module jdk.incubator.foreign

[Jonathan Gibbons]

Please review a simple fix to ensure that a link reference is resolved and 
displayed correctly.

-

Commit messages:
 - JDK-8262433: doclint: reference error in module jdk.incubator.foreign

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

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v2]

[Paul Sandoz]

On Thu, 25 Feb 2021 23:48:37 GMT, Jie Fu  wrote:

>> src/hotspot/share/prims/vectorSupport.cpp line 368:
>> 
>>> 366:   if (java_lang_Class::is_primitive(mirror)) {
>>> 367: BasicType bt = java_lang_Class::primitive_type(mirror);
>>> 368: int min_lane_count = 64 / type2aelembytes(bt);
>> 
>> I am uncertain of the units here. Is the numerator in bits and the 
>> denominator in bytes?
>
> Thanks Paul for your review.
> 
> Oops, the numerator should be 8 (bytes).
> 
> Updated.
> Testing of jdk/incubator/vector (MaxVector=default/8/4 or without C2) is 
> still fine.
> Thanks.

Thanks, was the test `VectorShapeInitTest` passing prior to the fix of the 
numerator?
Perhaps we should be testing more directly on 
`VectorShape.S_Max_BIT.vectorBitSize()` and `VectorShape.preferredShape` ? 
Also, perhaps we can run with C2 disabled, with `-XX:TieredStopAtLevel=3` ?

-

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

Re: RFR: JDK-8261839: Error creating runtime package on macos without mac-pack…

[Alexander Matveev]

On Thu, 25 Feb 2021 21:15:44 GMT, Andy Herrick  wrote:

> …age-identifier

Marked as reviewed by almatvee (Committer).

-

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v2]

[Jie Fu]

On Thu, 25 Feb 2021 17:23:11 GMT, Paul Sandoz  wrote:

>> Jie Fu has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   The numerator should be 8 (byte)
>
> src/hotspot/share/prims/vectorSupport.cpp line 368:
> 
>> 366:   if (java_lang_Class::is_primitive(mirror)) {
>> 367: BasicType bt = java_lang_Class::primitive_type(mirror);
>> 368: int min_lane_count = 64 / type2aelembytes(bt);
> 
> I am uncertain of the units here. Is the numerator in bits and the 
> denominator in bytes?

Thanks Paul for your review.

Oops, the numerator should be 8 (bytes).

Updated.
Testing of jdk/incubator/vector (MaxVector=default/8/4 or without C2) is still 
fine.
Thanks.

-

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception [v2]

[Jie Fu]

> Hi all,
> 
> Vector API fails to work when:
>  - case 1: MaxVectorSize is set to <=8, or
>  - case 2: C2 is disabled
> 
> The reason is that {max/preferred} VectorShape initialization fails in both 
> cases.
> And the root cause is that VectorSupport_GetMaxLaneCount [1] returns 
> unreasonable values (0 for case 1 and -1 for case 2).
> 
> Vector API should not depend on C2 to run.
> It should work even there is no JIT compiler since it's a Java-level api.
> So let's fix it.
> 
> Testing:
>   - jdk/incubator/vector with -XX:MaxVectorSize=default/8 on Linux/x64
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/vectorSupport.cpp#L364

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

  The numerator should be 8 (byte)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2722/files
  - new: https://git.openjdk.java.net/jdk/pull/2722/files/09b49f25..724b36d4

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

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

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

Integrated: JDK-8262428: doclint warnings in java.xml module

[Jonathan Gibbons]

On Thu, 25 Feb 2021 22:41:46 GMT, Jonathan Gibbons  wrote:

> Please review a small doc fix to remove some superfluous `` tags and an 
> erroneous `` tag, all reported by doclint..

This pull request has now been integrated.

Changeset: 059ede0d
Author:Jonathan Gibbons 
URL:   https://git.openjdk.java.net/jdk/commit/059ede0d
Stats: 7 lines in 1 file changed: 0 ins; 5 del; 2 mod

8262428: doclint warnings in java.xml module

Reviewed-by: bpb, lancea, naoto, iris

-

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

Re: RFR: JDK-8262428: doclint warnings in java.xml module

[Iris Clark]

On Thu, 25 Feb 2021 22:41:46 GMT, Jonathan Gibbons  wrote:

> Please review a small doc fix to remove some superfluous `` tags and an 
> erroneous `` tag, all reported by doclint..

Marked as reviewed by iris (Reviewer).

-

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

Re: RFR: JDK-8262428: doclint warnings in java.xml module

[Naoto Sato]

On Thu, 25 Feb 2021 22:41:46 GMT, Jonathan Gibbons  wrote:

> Please review a small doc fix to remove some superfluous `` tags and an 
> erroneous `` tag, all reported by doclint..

Marked as reviewed by naoto (Reviewer).

-

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

Re: RFR: JDK-8262428: doclint warnings in java.xml module

[Lance Andersen]

On Thu, 25 Feb 2021 22:41:46 GMT, Jonathan Gibbons  wrote:

> Please review a small doc fix to remove some superfluous `` tags and an 
> erroneous `` tag, all reported by doclint..

Looks OK.

Joe is in the process of updating module-info so  perhaps this can be combined 
with his change?

-

Marked as reviewed by lancea (Reviewer).

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

Re: RFR: JDK-8262428: doclint warnings in java.xml module

[Brian Burkhalter]

On Thu, 25 Feb 2021 22:41:46 GMT, Jonathan Gibbons  wrote:

> Please review a small doc fix to remove some superfluous `` tags and an 
> erroneous `` tag, all reported by doclint..

Looks fine.

-

Marked as reviewed by bpb (Reviewer).

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

RFR: JDK-8262428: doclint warnings in java.xml module

[Jonathan Gibbons]

Please review a small doc fix to remove some superfluous `` tags and an 
erroneous `` tag, all reported by doclint..

-

Commit messages:
 - JDK-8262428: doclint warnings in java.xml module

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

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

Re: RFR: 8261670: Add javadoc for the XML processing limits

[Joe Wang]

On Thu, 25 Feb 2021 22:33:02 GMT, Naoto Sato  wrote:

>> Add the documentation for XML processing limits to module summary. The 
>> limits were previously documented in Java tutorial and guide.
>
> src/java.xml/share/classes/module-info.java line 233:
> 
>> 231:  * If the value is not an integer, a NumericFormatException is thrown.
>> 232:  * 
>> 233:  * 64000
> 
> There are other ```Default``` ids in other rows. Should they be distinct? Or 
> rather needed?

True, not needed. Will remove, and also look at other copy ids.

-

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

Re: RFR: 8261670: Add javadoc for the XML processing limits

[Joe Wang]

On Thu, 25 Feb 2021 22:28:43 GMT, Naoto Sato  wrote:

>> Add the documentation for XML processing limits to module summary. The 
>> limits were previously documented in Java tutorial and guide.
>
> src/java.xml/share/classes/module-info.java line 231:
> 
>> 229:  * 
>> 230:  * A positive integer. A value less than or equal to 0 indicates no 
>> limit.
>> 231:  * If the value is not an integer, a NumericFormatException is thrown.
> 
> Is it a ```NumberFormatException```? Also, @link might be useful here.

Oh my, the typo has been in the tutorial all this time! It's 
NumberFormatException indeed.

-

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

Re: RFR: 8261670: Add javadoc for the XML processing limits

[Naoto Sato]

On Thu, 25 Feb 2021 22:04:41 GMT, Joe Wang  wrote:

> Add the documentation for XML processing limits to module summary. The limits 
> were previously documented in Java tutorial and guide.

src/java.xml/share/classes/module-info.java line 231:

> 229:  * 
> 230:  * A positive integer. A value less than or equal to 0 indicates no 
> limit.
> 231:  * If the value is not an integer, a NumericFormatException is thrown.

Is it a ```NumberFormatException```? Also, @link might be useful here.

src/java.xml/share/classes/module-info.java line 233:

> 231:  * If the value is not an integer, a NumericFormatException is thrown.
> 232:  * 
> 233:  * 64000

There are other ```Default``` ids in other rows. Should they be distinct? Or 
rather needed?

-

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

RFR: 8261670: Add javadoc for the XML processing limits

[Joe Wang]

Add the documentation for XML processing limits to module summary. The limits 
were previously documented in Java tutorial and guide.

-

Commit messages:
 - 8261670: Add javadoc for the XML processing limits

Changes: https://git.openjdk.java.net/jdk/pull/2732/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2732=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261670
  Stats: 87 lines in 1 file changed: 81 ins; 1 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2732.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2732/head:pull/2732

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

Re: RFR: JDK-8261839: Error creating runtime package on macos without mac-pack…

[Alexey Semenyuk]

On Thu, 25 Feb 2021 21:36:48 GMT, Alexey Semenyuk  wrote:

>> …age-identifier
>
> Marked as reviewed by asemenyuk (Committer).

Nice to have runtime packaging implemented on all supported platforms!

-

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

Re: RFR: JDK-8261839: Error creating runtime package on macos without mac-pack…

[Alexey Semenyuk]

On Thu, 25 Feb 2021 21:15:44 GMT, Andy Herrick  wrote:

> …age-identifier

Marked as reviewed by asemenyuk (Committer).

-

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

RFR: JDK-8261839: Error creating runtime package on macos without mac-pack…

[Andy Herrick]

…age-identifier

-

Commit messages:
 - JDK-8261839: Error creating runtime package on macos without 
mac-package-identifier

Changes: https://git.openjdk.java.net/jdk/pull/2730/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2730=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261839
  Stats: 24 lines in 4 files changed: 8 ins; 5 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2730.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2730/head:pull/2730

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

Re: JDK-8262003: Class.arrayType should not throw IllegalArgumentException

[Johannes Kuhn]

Thanks Mandy.

Will amend the CSR to cover TypeDescriptor.OfField::arrayType as well, 
and create a patch.


Where should I put the tests?
Didn't find any tests that exercise Class::arrayType in test/jdk/java/lang/.
Any wishes for tests that I should add w.r.t. Class::arrayType?

- Johannes

On 24-Feb-21 20:54, Mandy Chung wrote:

Hi Johannes,

Changing Class::arrayType to throw ISA makes sense to me.   I think 
`TypeDescriptor.ofField::arrayType` spec should also be updated to throw 
ISA to follow what JVM checks for array dimension because it's a static 
constraint check [1] (rather than a resolution error) for `anewarray` 
instruction to create an array no more than 255 dimensions.


The CSR looks okay.  I agree that the compatibility risk is low.

I'd like to review PR along with CSR together.

Mandy
[1] 
https://docs.oracle.com/javase/specs/jvms/se15/html/jvms-4.html#jvms-4.9.1


On 2/23/21 6:38 AM, Johannes Kuhn wrote:
I want to learn about writing a CSR, and need a sponsor teaching me 
the ropes.




Bug: https://bugs.openjdk.java.net/browse/JDK-8262003

Currently, Class.arrayType() will throw an IllegalArgumentException if 
the maximum number of dimensions will be exceeded.


Throwing an IllegalArgumentException from a method that doesn't take 
an argument is, at least, strange.


Therefore I would like to update the specification to allow this 
method to throw an IllegalStateException, similar to what 
ClassDesc.arrayType() does.




The current plan is:

* Create a CSR detailing the spec changes: 
https://bugs.openjdk.java.net/browse/JDK-8262211
* Surround the code with a try-catch block. Checking for the error 
case is hard, and somewhat rare, so I think this is appropriate.
* Copy the @throws javadoc line from ClassDesc.arrayType to 
Class.arrayType


But there are a few questions I would love to get the answer on:

* Both Class.arrayType() and ClassDesc.arrayType() are specified by 
TypeDescriptor.OfField. Should the specification of 
TypeDescriptor.OfField.arrayType() changed as well?
* Is the draft of my CSR fine? Should I add some details, or omit 
things? Rephrase things?


In advance, thanks for any support and feedback on this.

- Johannes

Re: JDK-8262003: Class.arrayType should not throw IllegalArgumentException

[Mandy Chung]

On 2/25/21 6:52 AM, Johannes Kuhn wrote:

Thanks Mandy.

Will amend the CSR to cover TypeDescriptor.OfField::arrayType as well, 
and create a patch.


Where should I put the tests?
Didn't find any tests that exercise Class::arrayType in 
test/jdk/java/lang/.

Any wishes for tests that I should add w.r.t. Class::arrayType?



Yes, I suggest to put it under test/jdk/java/lang/Class/arrayType.

Mandy


- Johannes

On 24-Feb-21 20:54, Mandy Chung wrote:

Hi Johannes,

Changing Class::arrayType to throw ISA makes sense to me.   I think 
`TypeDescriptor.ofField::arrayType` spec should also be updated to 
throw ISA to follow what JVM checks for array dimension because it's 
a static constraint check [1] (rather than a resolution error) for 
`anewarray` instruction to create an array no more than 255 dimensions.


The CSR looks okay.  I agree that the compatibility risk is low.

I'd like to review PR along with CSR together.

Mandy
[1] 
https://docs.oracle.com/javase/specs/jvms/se15/html/jvms-4.html#jvms-4.9.1


On 2/23/21 6:38 AM, Johannes Kuhn wrote:
I want to learn about writing a CSR, and need a sponsor teaching me 
the ropes.




Bug: https://bugs.openjdk.java.net/browse/JDK-8262003

Currently, Class.arrayType() will throw an IllegalArgumentException 
if the maximum number of dimensions will be exceeded.


Throwing an IllegalArgumentException from a method that doesn't take 
an argument is, at least, strange.


Therefore I would like to update the specification to allow this 
method to throw an IllegalStateException, similar to what 
ClassDesc.arrayType() does.




The current plan is:

* Create a CSR detailing the spec changes: 
https://bugs.openjdk.java.net/browse/JDK-8262211
* Surround the code with a try-catch block. Checking for the error 
case is hard, and somewhat rare, so I think this is appropriate.
* Copy the @throws javadoc line from ClassDesc.arrayType to 
Class.arrayType


But there are a few questions I would love to get the answer on:

* Both Class.arrayType() and ClassDesc.arrayType() are specified by 
TypeDescriptor.OfField. Should the specification of 
TypeDescriptor.OfField.arrayType() changed as well?
* Is the draft of my CSR fine? Should I add some details, or omit 
things? Rephrase things?


In advance, thanks for any support and feedback on this.

- Johannes

Re: RFR: JDK-8262199: issue in jli args.c [v3]

[Christoph Langer]

On Thu, 25 Feb 2021 16:39:25 GMT, Matthias Baesken  wrote:

>> src/java.base/share/native/libjli/args.c line 378:
>> 
>>> 376: if (st.st_size > MAX_ARGF_SIZE) {
>>> 377: JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
>>> 378: reportAndExit(NULL, NULL);
>> 
>> This should be just one statement,
>> reportAndExit(CFG_ERROR10, MAX_ARGF_SIZE);
>> or?
>
> I think that did not work because it does not fit the param types of 
> reportAndExit but I can simplify it otherwise.

Ah, I see.
Maybe it's a bit bike-sheddy but as you're using reportAndExit only twice now, 
wouldn't it be less convoluted if you use
JLI_ReportMessage(...);
exit(1);
inline in both places?

-

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

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v18]

[Gerard Ziemski]

On Wed, 17 Feb 2021 12:36:10 GMT, Anton Kozlov  wrote:

>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>> 
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
>> windows/aarch64. 
>> 
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with 
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
>> required on macOS/AArch64 platform. It's implemented with 
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
>> thread, so W^X mode change relates to the java thread state change (for java 
>> threads). In most cases, JVM executes in write-only mode, except when 
>> calling a generated stub like SafeFetch, which requires a temporary switch 
>> to execute-only mode. The same execute-only mode is enabled when a java 
>> thread executes in java or native states. This approach of managing W^X mode 
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 88 commits:
> 
>  - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos
>  - Re-do safefetch.hpp
>  - Merge remote-tracking branch 'origin/jdk/8261075-stubroutines-inline' into 
> jdk-macos
>  - stubRoutines.inline.hpp -> safefetch.hpp
>  - Update copyrights
>  - Merge remote-tracking branch 'upstream/jdk/master' into 
> 8261075-stubroutines-inline
>  - Merge remote-tracking branch 'upstream/jdk/master' into 
> 8261075-stubroutines-inline
>  - Extract SafeFetch32/N to stubRoutines.inline.hpp
>  - Revert "Extract SafeFetch32/N to stubRoutines.inline.hpp"
>
>This reverts commit b873c25f31dd21349d140b790713cc9ccb5f2dc0.
>  - Merge pull request #9 from VladimirKempik/pull/2200
>
>Removed unused variables
>  - ... and 78 more: 
> https://git.openjdk.java.net/jdk/compare/b955f85e...ab72613c

Marked as reviewed by gziemski (Committer).

-

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

Re: RFR: 8262096: Vector API fails to work due to VectorShape initialization exception

[Paul Sandoz]

On Thu, 25 Feb 2021 09:31:01 GMT, Jie Fu  wrote:

> Hi all,
> 
> Vector API fails to work when:
>  - case 1: MaxVectorSize is set to <=8, or
>  - case 2: C2 is disabled
> 
> The reason is that {max/preferred} VectorShape initialization fails in both 
> cases.
> And the root cause is that VectorSupport_GetMaxLaneCount [1] returns 
> unreasonable values (0 for case 1 and -1 for case 2).
> 
> Vector API should not depend on C2 to run.
> It should work even there is no JIT compiler since it's a Java-level api.
> So let's fix it.
> 
> Testing:
>   - jdk/incubator/vector with -XX:MaxVectorSize=default/8 on Linux/x64
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/vectorSupport.cpp#L364

src/hotspot/share/prims/vectorSupport.cpp line 368:

> 366:   if (java_lang_Class::is_primitive(mirror)) {
> 367: BasicType bt = java_lang_Class::primitive_type(mirror);
> 368: int min_lane_count = 64 / type2aelembytes(bt);

I am uncertain of the units here. Is the numerator in bits and the denominator 
in bytes?

-

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

Re: RFR: JDK-8262199: issue in jli args.c [v4]

[Alan Bateman]

On Thu, 25 Feb 2021 16:44:00 GMT, Matthias Baesken  wrote:

>> Sonar reports a finding in args.c, where a file check is done .
>> Stat performs a check on file, and later fopen is called on the file .
>> 
>> The coding could be slightly rewritten so that the potential issue is 
>> removed (however I do not think that it is such a big issue).
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Simplify coding

Thanks for the updates, the latest looks clean to me.

-

Marked as reviewed by alanb (Reviewer).

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

Re: RFR: JDK-8262199: issue in jli args.c [v3]

[Matthias Baesken]

On Thu, 25 Feb 2021 16:07:36 GMT, Christoph Langer  wrote:

>> Matthias Baesken has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Remove fclose before exit
>
> src/java.base/share/native/libjli/args.c line 378:
> 
>> 376: if (st.st_size > MAX_ARGF_SIZE) {
>> 377: JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
>> 378: reportAndExit(NULL, NULL);
> 
> This should be just one statement,
> reportAndExit(CFG_ERROR10, MAX_ARGF_SIZE);
> or?

I think that did not work because it does not fit the param types of 
reportAndExit but I can simplify it otherwise.

-

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

Re: RFR: JDK-8262199: issue in jli args.c [v4]

[Matthias Baesken]

> Sonar reports a finding in args.c, where a file check is done .
> Stat performs a check on file, and later fopen is called on the file .
> 
> The coding could be slightly rewritten so that the potential issue is removed 
> (however I do not think that it is such a big issue).

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

  Simplify coding

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2692/files
  - new: https://git.openjdk.java.net/jdk/pull/2692/files/b5eeca4f..39faea43

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

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

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

RFR: 8262379: Add regression test for JDK-8257746

[Severin Gehwolf]

Fails prior the patch of JDK-8257746, passes after. As expected.

Thoughts?

-

Commit messages:
 - 8262379: Add regression test for JDK-8257746

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

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

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

[Brian Burkhalter]

On Tue, 23 Feb 2021 18:00:33 GMT, Alan Bateman  wrote:

>> Brian Burkhalter has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader
>
> Marked as reviewed by alanb (Reviewer).

@AlanBateman, @dfuch, @RogerRiggs thanks for the reviews here and @jddarcy for 
approving the CSR!

-

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

Integrated: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

[Brian Burkhalter]

On Mon, 22 Feb 2021 23:27:19 GMT, Brian Burkhalter  wrote:

> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in 
> subclass overrides

This pull request has now been integrated.

Changeset: 5a9b7010
Author:Brian Burkhalter 
URL:   https://git.openjdk.java.net/jdk/commit/5a9b7010
Stats: 156 lines in 9 files changed: 41 ins; 55 del; 60 mod

8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in 
subclass overrides

Reviewed-by: alanb, rriggs

-

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

Re: RFR: JDK-8262199: issue in jli args.c [v3]

[Christoph Langer]

On Thu, 25 Feb 2021 15:40:00 GMT, Matthias Baesken  wrote:

>> Sonar reports a finding in args.c, where a file check is done .
>> Stat performs a check on file, and later fopen is called on the file .
>> 
>> The coding could be slightly rewritten so that the potential issue is 
>> removed (however I do not think that it is such a big issue).
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove fclose before exit

Changes requested by clanger (Reviewer).

src/java.base/share/native/libjli/args.c line 378:

> 376: if (st.st_size > MAX_ARGF_SIZE) {
> 377: JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
> 378: reportAndExit(NULL, NULL);

This should be just one statement,
reportAndExit(CFG_ERROR10, MAX_ARGF_SIZE);
or?

src/java.base/share/native/libjli/args.c line 358:

> 356: 
> 357: static void reportAndExit(const char* fmt, const char* arg) {
> 358: if (fmt != NULL) JLI_ReportMessage(fmt, arg);

the if (fmt != NULL) check wouldn't be necessary here if you fix the other 
location with reportAndExit(NULL, NULL), I think

-

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

Integrated: 8260403: javap should be more robust in the face of invalid class files

[Adam Sotona]

On Thu, 25 Feb 2021 13:01:30 GMT, Adam Sotona  wrote:

> Please review javap fix to handle java.lang.IllegalStateException for classes 
> with invalid Signature attribute.
> New test (T8260403) parsing class with invalid Signature attribute (as 
> described in the bug) is included.
> Fix wraps java.lang.IllegalStateException, reports "Error: Invalid value for 
> Signature attribute: " and continues.
> 
> Thanks,
> Adam

This pull request has now been integrated.

Changeset: 7d4f60b1
Author:Adam Sotona 
Committer: Jonathan Gibbons 
URL:   https://git.openjdk.java.net/jdk/commit/7d4f60b1
Stats: 165 lines in 3 files changed: 164 ins; 0 del; 1 mod

8260403: javap should be more robust in the face of invalid class files

Reviewed-by: vromero

-

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

Re: 8260403: javap should be more robust in the face of invalid class files

[Adam Sotona]

Hi Vicente,
thanks for the review.

May I ask you also to /sponsor this PR?

Thanks again,
Adam


On 25/02/2021, 16:39, "core-libs-dev on behalf of Vicente Romero" 
 
wrote:

On Thu, 25 Feb 2021 13:01:30 GMT, Adam Sotona  wrote:

> Please review javap fix to handle java.lang.IllegalStateException for 
classes with invalid Signature attribute.
> New test (T8260403) parsing class with invalid Signature attribute (as 
described in the bug) is included.
> Fix wraps java.lang.IllegalStateException, reports "Error: Invalid value 
for Signature attribute: " and continues.
> 
> Thanks,
> Adam

lgtm!

-

Marked as reviewed by vromero (Reviewer).

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

Re: RFR: 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with "AssertionError: Should have GCd a method handle by now" [v2]

[Aleksey Shipilev]

On Thu, 18 Feb 2021 19:31:02 GMT, Attila Szegedi  wrote:

>> 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with 
>> "AssertionError: Should have GCd a method handle by now"
>
> Attila Szegedi 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 good test would be trying with all current GCs (Serial, Parallel, G1, 
Shenandoah, ZGC):

make run-test TEST=jdk/dynalink TEST_VM_OPTS=-XX:+UseSerialGC

Also, make sure that GH actions are able to run this test. You probably need to 
enable the workflow here -- https://github.com/szegedi/jdk/actions -- and then 
trigger the workflow manually on your branch (or push another commit). Then 
"Checks" tab would show the results.

test/jdk/jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java line 28:

> 26:  * @bug 8198540
> 27:  * @summary Test TypeConverterFactory is not leaking method handles
> 28:  * @run main/othervm -Xmx4M TypeConverterFactoryMemoryLeakTest

I think `-Xmx4m` is risking it on some platforms that cannot go that low heap. 
Maybe do 128M, and bulk up the test allocations, so that GC definitely triggers?

test/jdk/jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java line 79:

> 77: 
> 78: public static void main(String[] args) {
> 79: for (int count = 0; count < MAX_ITERATIONS; ++count) {

Here and later: use postfix `count++`, regular style?

-

Changes requested by shade (Reviewer).

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

Re: RFR: JDK-8262199: issue in jli args.c [v3]

[Matthias Baesken]

> Sonar reports a finding in args.c, where a file check is done .
> Stat performs a check on file, and later fopen is called on the file .
> 
> The coding could be slightly rewritten so that the potential issue is removed 
> (however I do not think that it is such a big issue).

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

  Remove fclose before exit

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2692/files
  - new: https://git.openjdk.java.net/jdk/pull/2692/files/8b106a14..b5eeca4f

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

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

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

Re: RFR: 8260403: javap should be more robust in the face of invalid class files

[Vicente Romero]

On Thu, 25 Feb 2021 13:01:30 GMT, Adam Sotona  wrote:

> Please review javap fix to handle java.lang.IllegalStateException for classes 
> with invalid Signature attribute.
> New test (T8260403) parsing class with invalid Signature attribute (as 
> described in the bug) is included.
> Fix wraps java.lang.IllegalStateException, reports "Error: Invalid value for 
> Signature attribute: " and continues.
> 
> Thanks,
> Adam

lgtm!

-

Marked as reviewed by vromero (Reviewer).

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

Re: ZipInputStream#readAllBytes should clarify it doesn't read the whole stream?

[Jaikiran Pai]

Thank you Lance and Alan. I do have access to JBS, I'll file one 
tomorrow with the details.


-Jaikiran

On 25/02/21 9:04 pm, Lance Andersen wrote:

Hi Jaikiran,

Yes I believe this makes sense.

It would also require a CSR.

I can login a bug if you do not have access to do so.

Best
Lance

On Feb 24, 2021, at 10:56 PM, Jaikiran Pai > wrote:


The javadoc of InputStream#readAllBytes() states[1] that it reads all 
the remaining bytes of the stream. The java.util.zip.ZipInputStream 
doesn't override this method and thus "inherits" this javadoc. The 
implementation of InputStream#readAllBytes() ultimately ends up 
calling ZipInputStream#read()[2], so the implementation correctly 
reads only till the end of the current ZipEntry and not the entire 
ZipInputStream. However, because the javadoc gets inherited, reading 
any code like the following doesn't make it clear that it's only 
reading till the end of the current entry:


zis = ... // ZipInputStream
while ((e = zis.getNextEntry()) != null) {
    String name = e.getName();
    zis.readAllBytes(); // gives an impression that all bytes of the 
stream are read

    ...
}

Should the ZipInputStream override the readAllBytes(), just so as to 
add a very specific javadoc to this method which explains that it 
reads only till the end of current entry and other related semantics? 
Perhaps the same should be done for ZipInputStream#readNBytes(...)?



[1] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/io/InputStream.html#readAllBytes() 

[2] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/zip/ZipInputStream.html#read(byte%5B%5D,int,int) 



-Jaikiran






Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: ZipInputStream#readAllBytes should clarify it doesn't read the whole stream?

[Lance Andersen]

Hi Jaikiran,

Yes I believe this makes sense.

It would also require a CSR.

I can login a bug if you do not have access to do so.

Best
Lance

On Feb 24, 2021, at 10:56 PM, Jaikiran Pai 
mailto:jai.forums2...@gmail.com>> wrote:

The javadoc of InputStream#readAllBytes() states[1] that it reads all the 
remaining bytes of the stream. The java.util.zip.ZipInputStream doesn't 
override this method and thus "inherits" this javadoc. The implementation of 
InputStream#readAllBytes() ultimately ends up calling ZipInputStream#read()[2], 
so the implementation correctly reads only till the end of the current ZipEntry 
and not the entire ZipInputStream. However, because the javadoc gets inherited, 
reading any code like the following doesn't make it clear that it's only 
reading till the end of the current entry:

zis = ... // ZipInputStream
while ((e = zis.getNextEntry()) != null) {
String name = e.getName();
zis.readAllBytes(); // gives an impression that all bytes of the stream are 
read
...
}

Should the ZipInputStream override the readAllBytes(), just so as to add a very 
specific javadoc to this method which explains that it reads only till the end 
of current entry and other related semantics? Perhaps the same should be done 
for ZipInputStream#readNBytes(...)?


[1] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/io/InputStream.html#readAllBytes()
[2] 
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/zip/ZipInputStream.html#read(byte%5B%5D,int,int)

-Jaikiran


[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home]



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: ZipInputStream#readAllBytes should clarify it doesn't read the whole stream?

[Alan Bateman]

On 25/02/2021 03:56, Jaikiran Pai wrote:

:

Should the ZipInputStream override the readAllBytes(), just so as to 
add a very specific javadoc to this method which explains that it 
reads only till the end of current entry and other related semantics? 
Perhaps the same should be done for ZipInputStream#readNBytes(...)?
Make sense to avoid any confusion and also to do the readNBytes methods 
at the same time.


-Alan

Re: RFR: JDK-8262199: issue in jli args.c [v2]

[Alan Bateman]

On Thu, 25 Feb 2021 08:54:48 GMT, Matthias Baesken  wrote:

>> This function is "optionally report, optionally fclose, and then exit". Have 
>> you tried reducing it to reportAndExit and fclose inline in expandArgFile to 
>> avoid it doing 3 things?
>
>> This function is "optionally report, optionally fclose, and then exit". Have 
>> you tried reducing it to reportAndExit and fclose inline in expandArgFile to 
>> avoid it doing 3 things?
> 
> hi Alan ,  thanks for your remark ,  I did not do that because I would need 
> to copy both lines (fclose + exit)  to all callers.
> On the other hand I wonder -  wouldn't  it be possible  to avoid the fclose  
> before the  exit(1)  ?
> I assume the closing is done on exit anyway  ?

That sounds okay to me, the process is exiting, as you say.

-

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

RFR: 8260403: javap should be more robust in the face of invalid class files

[Adam Sotona]

Please review javap fix to handle java.lang.IllegalStateException for classes 
with invalid Signature attribute.
New test (T8260403) parsing class with invalid Signature attribute (as 
described in the bug) is included.
Fix wraps java.lang.IllegalStateException, reports "Error: Invalid value for 
Signature attribute: " and continues.

Thanks,
Adam

-

Commit messages:
 - 8260403: javap should be more robust in the face of invalid class files

Changes: https://git.openjdk.java.net/jdk/pull/2724/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2724=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260403
  Stats: 165 lines in 3 files changed: 164 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2724.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2724/head:pull/2724

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

Integrated: 8252883: AccessDeniedException caused by delayed file deletion on Windows

[Evan Whelan]

On Mon, 15 Feb 2021 10:39:32 GMT, Evan Whelan  wrote:

> Hi,
> 
> Please review this fix for JDK-8252883. This handles the case when an 
> AccessDeniedException is being thrown on Windows, due to a delay in deleting 
> the lock file it is trying to write to.
> 
> This fix passes all testing.
> 
> Kind regards,
> Evan

This pull request has now been integrated.

Changeset: ebdc80ea
Author:Evan Whelan 
Committer: Daniel Fuchs 
URL:   https://git.openjdk.java.net/jdk/commit/ebdc80ea
Stats: 73 lines in 2 files changed: 72 ins; 0 del; 1 mod

8252883: AccessDeniedException caused by delayed file deletion on Windows

Reviewed-by: dfuchs

-

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

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

[Сергей Цыпанов]

On Wed, 24 Feb 2021 08:50:36 GMT, Alan Bateman  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8261880: Remove static from declarations of Holder nested classes
>
> src/java.base/windows/classes/sun/nio/ch/PipeImpl.java line 67:
> 
>> 65: private final SinkChannel sink;
>> 66: 
>> 67: private static class Initializer
> 
> This one is okay to do.

Thanks for review! Could you review the rest of the code and approve this PR, 
if it's fine?

-

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

Re: RFR: JDK-8262199: issue in jli args.c [v2]

[Matthias Baesken]

On Tue, 23 Feb 2021 19:24:31 GMT, Alan Bateman  wrote:

> This function is "optionally report, optionally fclose, and then exit". Have 
> you tried reducing it to reportAndExit and fclose inline in expandArgFile to 
> avoid it doing 3 things?

hi Alan ,  thanks for your remark ,  I did not do that because I would need to 
copy both lines (fclose + exit)  to all callers.
On the other hand I wonder -  wouldn't  it be possible  to avoid the fclose  
before the  exit(1)  ?
I assume the closing is done on exit anyway  ?

-

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