Re: RFR: 8321400: java/foreign/TestStubAllocFailure.java fails with code cache exhaustion

2023-12-12 Thread David Holmes
On Mon, 11 Dec 2023 13:01:25 GMT, Jorn Vernee wrote: > Improve the test by being more lenient to related code cache exhaustion > errors. The important thing is that we don't terminate with a fatal error, > which the new code now checks for explicitly. The check for that is based on > what is

Re: RFR: 8320919: Clarify Locale related system properties [v3]

2023-12-12 Thread Stuart Marks
On Mon, 11 Dec 2023 23:20:25 GMT, Naoto Sato wrote: >> This is a doc change to clarify what the `Default Locale` is, and how it is >> established during the system startup using the system properties. Those >> locale-related system properties have existed since the early days of Java, >> but

Re: RFR: 8320919: Clarify Locale related system properties [v3]

2023-12-12 Thread Stuart Marks
On Mon, 11 Dec 2023 23:17:01 GMT, Naoto Sato wrote: >> src/java.base/share/classes/java/util/Locale.java line 301: >> >>> 299: * is unparsable, it is ignored. The overriding values of other >>> properties are not >>> 300: * checked for syntax or validity and are used directly in the default

Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577

2023-12-12 Thread Guoxiong Li
On Tue, 12 Dec 2023 15:40:19 GMT, Magnus Ihse Bursie wrote: > Somehow the JDK compiled with 7.5 for the author of that patch, but fails for > the author of this patch. I don't understand how this both can be true. I noticed my previous build is `slowdebug`. Today, I tested the `release` and

Re: RFR: 8321206: Make Locale related system properties `StaticProperty` [v2]

2023-12-12 Thread Ioi Lam
On Wed, 6 Dec 2023 20:55:48 GMT, Naoto Sato wrote: >> Currently, Locale-related system properties, such as `user.language` or >> `user.country`, are initialized when the `Locale` class is loaded. Making >> them static properties is safer than relying on the `Locale` class loading >> timing,

Re: RFR: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable [v3]

2023-12-12 Thread Leonid Mesnik
On Fri, 8 Dec 2023 11:54:40 GMT, Serguei Spitsyn wrote: >> This fix is for JDK 23 but the intention is to back port it to 22 in RDP-1 >> time frame. >> It is fixing a deadlock issue between `VirtualThread` class critical >> sections with the `interruptLock` (in methods: `unpark()`,

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v4]

2023-12-12 Thread Brent Christian
On Wed, 6 Dec 2023 01:24:51 GMT, Brent Christian wrote: >> Perhaps in each of these places add a link to #Memory Consistency Properties > > I can flesh out the new Memory Consistency Properties section in the package > info to cover the whole chain. Adding links to it sounds like a good idea.

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v4]

2023-12-12 Thread Brent Christian
> Classes in the `java.lang.ref` package would benefit from an update to bring > the spec in line with how the VM already behaves. The changes would focus on > _happens-before_ edges at some key points during reference processing. > > A couple key things we want to be able to say are: > -

Re: RFR: 8321206: Make Locale related system properties `StaticProperty` [v2]

2023-12-12 Thread David Holmes
On Wed, 6 Dec 2023 20:55:48 GMT, Naoto Sato wrote: >> Currently, Locale-related system properties, such as `user.language` or >> `user.country`, are initialized when the `Locale` class is loaded. Making >> them static properties is safer than relying on the `Locale` class loading >> timing,

Re: RFR: 8321713: Harmonize executeTestJvm with create[Limited]TestJavaProcessBuilder [v3]

2023-12-12 Thread Leonid Mesnik
On Mon, 11 Dec 2023 14:06:43 GMT, Stefan Karlsson wrote: >> [JDK-8315097](https://bugs.openjdk.org/browse/JDK-8315097): 'Rename >> createJavaProcessBuilder' changed the name of the ProcessTools helper >> functions used to create `ProcessBuilder`s used to spawn new java test >> processes. >>

Re: RFR: 8320575: generic type information lost on mandated parameters [v7]

2023-12-12 Thread Joe Darcy
On Tue, 12 Dec 2023 04:26:25 GMT, Vicente Romero wrote: >> Reflection is not retrieving generic type information for mandated >> parameters. This is a known issue which has been explicitly stated in the >> API of some reflection methods. Fix for >>

Re: RFR: 8320575: generic type information lost on mandated parameters [v7]

2023-12-12 Thread Joe Darcy
On Tue, 12 Dec 2023 22:30:02 GMT, Joe Darcy wrote: > It may be helpful to link to javax.lang.model.util.Elements.Origin.MANDATED > and javax.lang.model.util.Elements.Origin.SYNTHETIC to give the reader some > more context here. If you do that, you may need to add some SuppressWarnings > to

Re: RFR: 8320575: generic type information lost on mandated parameters [v7]

2023-12-12 Thread Joe Darcy
On Tue, 12 Dec 2023 04:26:25 GMT, Vicente Romero wrote: >> Reflection is not retrieving generic type information for mandated >> parameters. This is a known issue which has been explicitly stated in the >> API of some reflection methods. Fix for >>

Re: ZipEntry

2023-12-12 Thread Lance Andersen
Please refer to JDK-8250968 which why the field in question was added. If you are interested in additional support, you can open an enhancement request for consideration. Best Lance On Dec 12, 2023, at 4:15 PM, Alan Snyder

Re: RFR: 8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled [v2]

2023-12-12 Thread Roger Riggs
On Tue, 12 Dec 2023 10:57:20 GMT, Aleksei Voitylov wrote: >> Aleksei Voitylov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review comments > > Thanks Roger. I agree with your point and pulled your suggested test in the > PR.

Re: ZipEntry

2023-12-12 Thread Alan Snyder
> On Dec 12, 2023, at 12:57 PM, Eirik Bjørsnøs wrote: > > Alan, > > ZipOutputStream is a relatively high-level API for producing ZIP files. There > are many shapes and corner cases allowed by APPNOTE.TXT which ZipOutputStream > has no means to produce. So while exposing the "external file

Integrated: 8321180: Condition for non-latin1 string size too large exception is off by one

2023-12-12 Thread Roger Riggs
On Wed, 6 Dec 2023 23:31:26 GMT, Roger Riggs wrote: > In the compact string implementation of non-latin1 (UTF16) strings the length > is constrained by VM implementation limit on the size a byte array that can > be allocated. To produce a useful exception the implementation checks the >

Re: ZipEntry

2023-12-12 Thread Eirik Bjørsnøs
Alan, ZipOutputStream is a relatively high-level API for producing ZIP files. There are many shapes and corner cases allowed by APPNOTE.TXT which ZipOutputStream has no means to produce. So while exposing the "external file attributes" field (which was incorrectly named "extraAttributes" in

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v9]

2023-12-12 Thread Sergey Tsypanov
On Mon, 11 Dec 2023 19:53:40 GMT, Sergey Tsypanov wrote: >> src/java.base/share/classes/java/io/IOStreams.java line 1: >> >>> 1: /* >> >> Please fix Whitespace errors. Thanks. :-) > > I've just copy-pasted the header from another class. How should I fix it? Done - PR Review

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v12]

2023-12-12 Thread Sergey Tsypanov
> It looks like we can skip copying of `byte[]` in > `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in > `java.io`. > > See comment by @vlsi in > https://github.com/openjdk/jdk/pull/10525/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1R612

ZipEntry

2023-12-12 Thread Alan Snyder
ZipEntry is a public class and I am aware that it is used outside the JDK. Presumably that is not a problem. I’m wondering why the class stores the external file attributes field but does not provide public accessors for it. I would find it useful to have access to this field. I would rather

Re: RFR: 8321480: ISO 4217 Amendment 176 Update [v3]

2023-12-12 Thread Naoto Sato
On Mon, 11 Dec 2023 22:15:42 GMT, Justin Lu wrote: >> Please review this PR which incorporates the ISO 4217 Amendment 176 Update. >> As the replacement of `ANG` to `XCG` won't occur until 2025, this change >> does not need to go into JDK22. `HR` was also updated to remove the past >> cutover

Integrated: 6230751: [Fmt-Ch] Recursive MessageFormats in ChoiceFormats ignore indicated subformats

2023-12-12 Thread Justin Lu
On Wed, 29 Nov 2023 22:41:26 GMT, Justin Lu wrote: > Please review this PR which updates an incorrect code example in > _java/text/ChoiceFormat_. > > ChoiceFormat (and MessageFormat) provide an example of how to produce a > pattern that supports singular and plural forms. The ChoiceFormat

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v3]

2023-12-12 Thread Brent Christian
> Classes in the `java.lang.ref` package would benefit from an update to bring > the spec in line with how the VM already behaves. The changes would focus on > _happens-before_ edges at some key points during reference processing. > > A couple key things we want to be able to say are: > -

Re: RFR: 8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled [v2]

2023-12-12 Thread Roger Riggs
On Tue, 12 Dec 2023 10:47:48 GMT, Aleksei Voitylov wrote: >> Since JDK-8311906, if CompactStrings is not enabled, index is not considered >> when calling extractCodepoints from StringUTF16.toBytes(). Because of that >> the last elements of the source codepoints array are stripped from the >>

Re: RFR: 8319577: x86_64 AVX2 intrinsics for Arrays.sort methods (int, float arrays) [v12]

2023-12-12 Thread Srinivas Vamsi Parasa
On Tue, 12 Dec 2023 15:42:09 GMT, Magnus Ihse Bursie wrote: >> Thank you Magnus! > > @vamsi-parasa You said: >> Made sure that OpenJDK builds without errors using both GCC 7.5 and GCC 6.4. > > but now we have https://bugs.openjdk.org/browse/JDK-8321688. Did you > introduce any changes after

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v7]

2023-12-12 Thread Per Minborg
> This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to fix a bug where the allocation size would overflow >

Re: RFR: 8319577: x86_64 AVX2 intrinsics for Arrays.sort methods (int, float arrays) [v12]

2023-12-12 Thread Magnus Ihse Bursie
On Wed, 6 Dec 2023 17:20:10 GMT, Srinivas Vamsi Parasa wrote: >> Okay, then I guess I am fine with this. > > Thank you Magnus! @vamsi-parasa You said: > Made sure that OpenJDK builds without errors using both GCC 7.5 and GCC 6.4. but now we have https://bugs.openjdk.org/browse/JDK-8321688. Did

Re: RFR: 8321688: Build on linux with GCC 7.5.0 fails after 8319577

2023-12-12 Thread Magnus Ihse Bursie
On Tue, 12 Dec 2023 02:31:58 GMT, Kim Barrett wrote: >> Hi all, >> >> This patch fixes the building failure introduced by >> [JDK-8319577](https://bugs.openjdk.org/browse/JDK-8319577) in old GCC >> version (linux & GCC 7.5.0 locally). >> >> Thanks for the review. >> >> Best Regards, >> --

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v6]

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 15:29:46 GMT, Per Minborg wrote: >> This PR proposes to change the specification for some methods that take >> `long` offsets so that they will throw an `IllegalArgumentException` rather >> than an `IndexOutOfBoundsException` for negative values. >> >> The PR also proposes

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v6]

2023-12-12 Thread Per Minborg
> This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to fix a bug where the allocation size would overflow >

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v5]

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 15:09:45 GMT, Per Minborg wrote: >> This PR proposes to change the specification for some methods that take >> `long` offsets so that they will throw an `IllegalArgumentException` rather >> than an `IndexOutOfBoundsException` for negative values. >> >> The PR also proposes

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v5]

2023-12-12 Thread Per Minborg
> This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to fix a bug where the allocation size would overflow >

Re: RFR: 8321400: java/foreign/TestStubAllocFailure.java fails with code cache exhaustion

2023-12-12 Thread Jorn Vernee
On Tue, 12 Dec 2023 12:09:50 GMT, Maurizio Cimadamore wrote: >> Improve the test by being more lenient to related code cache exhaustion >> errors. The important thing is that we don't terminate with a fatal error, >> which the new code now checks for explicitly. The check for that is based on

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v4]

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 14:01:49 GMT, Maurizio Cimadamore wrote: >> Per Minborg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert change in allocateNoInit > >

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v4]

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 13:58:42 GMT, Maurizio Cimadamore wrote: >> Per Minborg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert change in allocateNoInit > > src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v4]

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 13:23:27 GMT, Per Minborg wrote: >> This PR proposes to change the specification for some methods that take >> `long` offsets so that they will throw an `IllegalArgumentException` rather >> than an `IndexOutOfBoundsException` for negative values. >> >> The PR also proposes

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v4]

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 13:23:27 GMT, Per Minborg wrote: >> This PR proposes to change the specification for some methods that take >> `long` offsets so that they will throw an `IllegalArgumentException` rather >> than an `IndexOutOfBoundsException` for negative values. >> >> The PR also proposes

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v4]

2023-12-12 Thread Per Minborg
> This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to fix a bug where the allocation size would overflow >

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v3]

2023-12-12 Thread Per Minborg
> This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to fix a bug where the allocation size would overflow >

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment, ValueLayout, long, long) spec mismatch in exception scenario [v2]

2023-12-12 Thread Per Minborg
> This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to fix a bug where the allocation size would overflow >

Re: RFR: 8321400: java/foreign/TestStubAllocFailure.java fails with code cache exhaustion

2023-12-12 Thread Maurizio Cimadamore
On Mon, 11 Dec 2023 13:01:25 GMT, Jorn Vernee wrote: > Improve the test by being more lenient to related code cache exhaustion > errors. The important thing is that we don't terminate with a fatal error, > which the new code now checks for explicitly. The check for that is based on > what is

Re: RFR: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem [v2]

2023-12-12 Thread Lance Andersen
On Tue, 12 Dec 2023 11:59:36 GMT, Eirik Bjorsnos wrote: >> Please review this PR which adds validation of incorrect LOC signatures in >> `ZipFileSystem`. >> >> `ZipFile` already rejects the case where the offset pointed to from the CEN >> header does not start with the expected LOC

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 11:49:17 GMT, Per Minborg wrote: >> True. But we will get the wrong type of exception. > > We could change the exception type to `ArithmeticException` in this case and > get performance and correctness at the same time. I simply don't think we should be using

Re: RFR: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem [v2]

2023-12-12 Thread Eirik Bjorsnos
On Tue, 12 Dec 2023 11:47:35 GMT, Lance Andersen wrote: >> Eirik Bjorsnos has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use uppercase LOC in ZipException messages > > src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line

Re: RFR: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem [v2]

2023-12-12 Thread Eirik Bjorsnos
> Please review this PR which adds validation of incorrect LOC signatures in > `ZipFileSystem`. > > `ZipFile` already rejects the case where the offset pointed to from the CEN > header does not start with the expected LOC signature. It makes sense to add > this check to `ZipFileSystem` as

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario

2023-12-12 Thread Per Minborg
On Tue, 12 Dec 2023 11:45:22 GMT, Per Minborg wrote: >> src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 737: >> >>> 735: private MemorySegment allocateNoInit(MemoryLayout layout, long >>> size) { >>> 736: long byteSize; >>> 737: try { >> >> I don't

Re: RFR: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem

2023-12-12 Thread Lance Andersen
On Mon, 11 Dec 2023 15:38:28 GMT, Eirik Bjorsnos wrote: > Please review this PR which adds validation of incorrect LOC signatures in > `ZipFileSystem`. > > `ZipFile` already rejects the case where the offset pointed to from the CEN > header does not start with the expected LOC signature. It

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario

2023-12-12 Thread Per Minborg
On Tue, 12 Dec 2023 11:05:44 GMT, Maurizio Cimadamore wrote: >> This PR proposes to change the specification for some methods that take >> `long` offsets so that they will throw an `IllegalArgumentException` rather >> than an `IndexOutOfBoundsException` for negative values. >> >> The PR also

Integrated: JDK-8321889: JavaDoc method references with wrong (nested) type

2023-12-12 Thread Hannes Wallnöfer
On Tue, 12 Dec 2023 09:46:29 GMT, Hannes Wallnöfer wrote: > Please review a doc-only change for JavaDoc references using a nested class > instead of the enclosing class containing the target method. Until now this > is accepted by JavaDoc, but with >

Re: RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario

2023-12-12 Thread Maurizio Cimadamore
On Tue, 12 Dec 2023 10:02:00 GMT, Per Minborg wrote: > This PR proposes to change the specification for some methods that take > `long` offsets so that they will throw an `IllegalArgumentException` rather > than an `IndexOutOfBoundsException` for negative values. > > The PR also proposes to

Re: RFR: 8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled [v2]

2023-12-12 Thread Aleksei Voitylov
On Tue, 12 Dec 2023 10:47:48 GMT, Aleksei Voitylov wrote: >> Since JDK-8311906, if CompactStrings is not enabled, index is not considered >> when calling extractCodepoints from StringUTF16.toBytes(). Because of that >> the last elements of the source codepoints array are stripped from the >>

Re: RFR: JDK-8321889: JavaDoc method references with wrong (nested) type [v2]

2023-12-12 Thread Alan Bateman
On Tue, 12 Dec 2023 10:42:32 GMT, Hannes Wallnöfer wrote: >> Please review a doc-only change for JavaDoc references using a nested class >> instead of the enclosing class containing the target method. Until now this >> is accepted by JavaDoc, but with >>

Re: RFR: 8321514: UTF16 string gets constructed incorrectly from codepoints if CompactStrings is not enabled [v2]

2023-12-12 Thread Aleksei Voitylov
> Since JDK-8311906, if CompactStrings is not enabled, index is not considered > when calling extractCodepoints from StringUTF16.toBytes(). Because of that > the last elements of the source codepoints array are stripped from the > resulting UTF16 string, which fires in other places (e.g. during

Re: RFR: JDK-8321889: JavaDoc method references with wrong (nested) type [v2]

2023-12-12 Thread Hannes Wallnöfer
> Please review a doc-only change for JavaDoc references using a nested class > instead of the enclosing class containing the target method. Until now this > is accepted by JavaDoc, but with > [JDK-8164094](https://bugs.openjdk.org/browse/JDK-8164094) these references > are reported as errors.

RFR: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario

2023-12-12 Thread Per Minborg
This PR proposes to change the specification for some methods that take `long` offsets so that they will throw an `IllegalArgumentException` rather than an `IndexOutOfBoundsException` for negative values. The PR also proposes to fix a bug where the allocation size would overflow and the

RFR: JDK-8321889: JavaDoc method references with wrong (nested) type

2023-12-12 Thread Hannes Wallnöfer
Please review a change to fix JavaDoc references using a nested class instead of the enclosing class containing the target method. Until now this is accepted by JavaDoc (with the correct link being created). With [JDK-8164094](https://bugs.openjdk.org/browse/JDK-8164094) these references are

RFR: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem

2023-12-12 Thread Eirik Bjorsnos
Please review this PR which adds validation of incorrect LOC signatures in `ZipFileSystem`. `ZipFile` already rejects the case where the offset pointed to from the CEN header does not start with the expected LOC signature. It makes sense to add this check to `ZipFileSystem` as well.

Re: RFR: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem

2023-12-12 Thread Alan Bateman
On Mon, 11 Dec 2023 15:38:28 GMT, Eirik Bjorsnos wrote: > Please review this PR which adds validation of incorrect LOC signatures in > `ZipFileSystem`. > > `ZipFile` already rejects the case where the offset pointed to from the CEN > header does not start with the expected LOC signature. It

Re: RFR: JDK-8319626: Override toString() for ZipFile [v8]

2023-12-12 Thread Jaikiran Pai
On Mon, 11 Dec 2023 05:47:33 GMT, Justin Lu wrote: >> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8319982) >> which overrides and provides an implementation of `toString()` in >> _java.util.zip.ZipFile_ (and by extension, _java.util.jar.JarFile_). > > Justin Lu has

Re: RFR: JDK-8319626: Override toString() for ZipFile [v7]

2023-12-12 Thread Jaikiran Pai
On Mon, 11 Dec 2023 05:44:38 GMT, Justin Lu wrote: >> src/java.base/share/classes/java/util/zip/ZipFile.java line 494: >> >>> 492: @Override >>> 493: public String toString() { >>> 494: return res.zsrc.key.file.getName() >> >> Hello Justin, relying on `res.zsrc.key.file` to get

Re: RFR: 8321718: ProcessTools.executeProcess calls waitFor before logging

2023-12-12 Thread David Holmes
On Tue, 12 Dec 2023 08:57:08 GMT, Stefan Karlsson wrote: > Do you agree that it would be OK to remove the test? Sure that's fine. - PR Comment: https://git.openjdk.org/jdk/pull/17052#issuecomment-1851644385

Re: RFR: 8321718: ProcessTools.executeProcess calls waitFor before logging [v2]

2023-12-12 Thread Stefan Karlsson
> There is some logging printed when tests spawns processes. This logging is > triggered from calls to `OutputAnalyzer`, when it delegates calls to > `LazyOutputBuffer`. > > If we write code like this: > > ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder(...); > OutputAnalyzer

Re: RFR: 8321718: ProcessTools.executeProcess calls waitFor before logging [v2]

2023-12-12 Thread Stefan Karlsson
On Tue, 12 Dec 2023 07:43:31 GMT, David Holmes wrote: >> Stefan Karlsson has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Typo > > test/lib/jdk/test/lib/process/OutputAnalyzer.java line 111: > >> 109: /** >> 110: * Delegate

Re: RFR: 8321718: ProcessTools.executeProcess calls waitFor before logging

2023-12-12 Thread Stefan Karlsson
On Mon, 11 Dec 2023 11:16:05 GMT, Stefan Karlsson wrote: > There is some logging printed when tests spawns processes. This logging is > triggered from calls to `OutputAnalyzer`, when it delegates calls to > `LazyOutputBuffer`. > > If we write code like this: > > ProcessBuilder pb =