Re: RFR: 8273682: Upgrade Jline to 3.20.0

2021-10-11 Thread Athijegannathan Sundararajan
On Thu, 23 Sep 2021 14:43:21 GMT, Jan Lahoda  wrote:

> I'd like to upgrade the internal JLine to 3.20.0, to support the rxvt 
> terminal (see JDK-8270943), and to generally use a newer version of the 
> library. This patch is basically a application of relevant parts of the diff 
> between JLine 3.14.0 and 3.20.0, with merge fixes as needed.
> 
> Thanks!

LGTM

src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/CompletionMatcher.java
 line 32:

> 30:  *
> 31:  * @param candidates list of candidates
> 32:  * @return a map of candidates that completion matcher matches

a list of ..?

src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/EndOfFileException.java
 line 2:

> 1: /*
> 2:  * Copyright (c) 2002-2020, the original author or authors.

2021

-

Marked as reviewed by sundar (Reviewer).

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


Re: RFR: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes

2021-10-11 Thread Andrey Turbanov
On Thu, 7 Oct 2021 16:48:06 GMT, Naoto Sato  wrote:

>> StringBuffer is a legacy synchronized class. There are more modern 
>> alternatives which perform better:
>> 1. Plain String concatenation should be preferred
>> 2. StringBuilder is a direct replacement to StringBuffer which generally 
>> have better performance
>> 
>> In [JDK-8264029](https://bugs.openjdk.java.net/browse/JDK-8264029)  I 
>> migrated only usages which were automatically detected by IDEA. It turns out 
>> there are more usages which can be migrated.
>
> src/java.base/share/classes/java/lang/Character.java line 123:
> 
>> 121:  * than U+ are called supplementary characters.  The Java
>> 122:  * platform uses the UTF-16 representation in {@code char} arrays and
>> 123:  * in the {@code String} and {@code StringBuilder} classes. In
> 
> Not sure simple replacement applies here, as `StringBuffer` still uses 
> `UTF-16` representation. You may add `StringBuilder` as well here, but I 
> think you might want to file a CSR to clarify.

reverted changes in this spec.

-

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


Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v3]

2021-10-11 Thread Andrey Turbanov
> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE

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

  8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
  avoid link in private javadoc

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5878/files
  - new: https://git.openjdk.java.net/jdk/pull/5878/files/4ba785b3..d35afde5

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

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

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


RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior

2021-10-11 Thread Mandy Chung
Classes compiled prior to the nestmate support will generate 
`REF_invokeSpecial` if the implementation method is a private instance method.  
 Since a lambda proxy class is a hidden class, a nestmate of the host class, it 
can invoke the private implementation method but it has to use 
`REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the old 
classes running on the new runtime, LMF implementation adjusts the reference 
kind from `REF_invokeSpecial` to `REF_invokeVirtual/REF_invokeInterface`. 

This PR fixes the check properly to ensure the adjustment is only made if the 
implementation method is private method in the host class.

-

Commit messages:
 - JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method 
has incorrect behavior

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

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


Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]

2021-10-11 Thread Martin Buchholz
On Mon, 11 Oct 2021 18:52:07 GMT, Andrey Turbanov  wrote:

>> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>   update javadoc of 'newCapacity' method to refer 
> ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead

We generally avoid  in javadoc.
Especially in private javadoc.
I would write this more simply/readably as

* {@code SOFT_MAX_ARRAY_LENGTH >> coder}

-

Marked as reviewed by martin (Reviewer).

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


Re: RFR: 8271737: Only normalize the cached user.dir property once

2021-10-11 Thread Paul Hohensee
On Thu, 7 Oct 2021 14:05:19 GMT, Evgeny Astigeevich  
wrote:

> The change completes the fix of 
> [JDK-8198997](https://bugs.openjdk.java.net/browse/JDK-8198997) which has 
> added normalisation in a constructor but not removed it from the get method.

Lgtm.

-

Marked as reviewed by phh (Reviewer).

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


Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

2021-10-11 Thread Remi Forax
I agree with the idea of a try() syntax, but i don't think we need more 
interfaces.

Yes, John is right about the fact that the TWR Aucloseable does not work well 
with checked exceptions,
but the issue is more that there is nothing that works well with checked 
exceptions because Java has no way to compose them correctly
(remainder: we should remove the idea of checked exceptions from the language 
given that it's a backward compatible change and Kotlin, C# are safer than Java 
because users of those languages are not used to write a 
try/catch/printStackTrace in those languages unlike in Java).

So for me, AutoCloseable is enough.

The problem is more than a fluent API are expression oriented and a 
try-with-resources is statement oriented, hence the mismatch.

Like we have introduced the switch expression, here we need a 
try-with-resources expression.

There are good reasons to have a try-with-resources expression
1) it can be used to track a fluent chain that should close() the resources
2) the compiler can guarantee that the stream/reader/etc around the resources 
does not leak outside.

In term of syntax, i don't think that the arrow syntax (the one used for a case 
of a switch case or a lambda) should be used here, because using a block 
expression seems to be always a mistake.

I like the proposal from John, a try(expression) or a try expression (without 
the parenthesis).

regards,
Rémi


- Original Message -
> From: "John Rose" 
> To: "Paul Sandoz" , "Brian Goetz" 
> , "Tagir F.Valeev"
> 
> Cc: "core-libs-dev" 
> Sent: Lundi 11 Octobre 2021 20:42:20
> Subject: Re: RFR: 8274412: Add a method to Stream API to consume and close 
> the stream without using try-with-resources

> So the purpose of TWR is to hold an object with a “close-debt”
> (debt of a future call to close) and pay it at the end of a block,
> sort of like C++ RAII (but also sort of not).
> 
> But fluent syntaxes (which I like very much and hope to see
> more of in the future!) don’t play well with blocks, so if a
> fluent chain (any part of that chain:  It’s multiple objects)
> incurs a “close-debt”, it’s hard to jam a TWR block into it.
> 
> Hence the current proposal.  I agree with Brian and Paul
> that we haven’t examined all the corners of this problem
> yet.  And I’d like to poke at the concept of “close-debt” to
> help with the examination.
> 
> Just for brain storming, I think we could model “close-debt”
> outside either fluent API structure or TWR block structure.
> Java O-O APIs are the pre-eminent way to model things in
> Java, and they work exceedingly well, when used with skill.
> 
> AutoCloseable models close-debt of course.  But it has two
> weaknesses:  It doesn’t model anything *other* than the
> debt, and its (sole) method skates awkwardly around the
> issue of checked exceptions.  (It requires an override with
> exception type narrowing to be used in polite company.)
> AC is more of an integration hook with TWR, rather than
> a general-purpose model for close-debt.  Therefore it doesn’t
> teach us much about close-debt in a fluent setting.
> 
> Surely we can model close-debt better.  Let’s say that an
> operation (expression) with close-debt *also* has a return
> value and (for grins) *also* has an exception it might throw.
> This gets us to an API closer to Optional.  (If you hear the
> noise of a monad snuffling around in the dark outside
> your tent, you are not wrong.)
> 
> interface MustClose_1 {
>   T get() throws X;  //or could throw some Y or nothing at all
>   void close() throws X;
> }
> 
> (I wish we had an easier way to associate such an X
> with such a T, so that Stream could be more
> interoperable with simple Stream.  It’s a pain to
> carry around those X arguments.  So I’ll drop X now.)
> 
> interface MustClose_2 {
>   T get();
>   void close() throws Exception;
> }
> 
> An expression of this type requires (in general) two
> operations to finish up:  It must be closed, and the result
> (type T) must be gotten.  There’s an issue of coupling between
> the two methods; I would say, decouple their order, so that
> the “get” call, as with Optional, can be done any time,
> and the “close” call can be done in any order relative
> to “get”.  Both calls should be idempotent, I think.
> But that’s all second-order detail.
> 
> A first-order detail is the apparent but incorrect 1-1 relation
> between T values and close-debts.  That’s very wrong;
> a closable stream on 1,000 values has one close-debt,
> not 1,000.  So maybe we need:
> 
> interface MustClose_3 {
>   S map(Function value);
>   void close() throws Exception;
> }
> 
> That “map” method looks a little like Remi’s apply
> method.  Did I mention this design requires skill
> (as well as flexibility, with one hand already tied
> by checked exceptions)?  I’m at the edge of my own
> skill here, but I think there’s good ground to explore
> here.
> 
> In a fluent setting, a Stream that incurs a close-debt
> might be typed (after incurring the 

Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]

2021-10-11 Thread Pavel Rappo
On Mon, 11 Oct 2021 18:52:07 GMT, Andrey Turbanov  wrote:

>> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>   update javadoc of 'newCapacity' method to refer 
> ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead

I'll run tests; if they pass, I'll sponsor the change.

-

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


Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]

2021-10-11 Thread Andrey Turbanov
> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE

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

  275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
  update javadoc of 'newCapacity' method to refer 
ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5878/files
  - new: https://git.openjdk.java.net/jdk/pull/5878/files/d679bd3a..4ba785b3

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

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

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


Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]

2021-10-11 Thread Andrey Turbanov
On Sun, 10 Oct 2021 22:13:29 GMT, Sergey Bylokhov  wrote:

>> Andrey Turbanov has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>>   update javadoc of 'newCapacity' method to refer 
>> ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead
>
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 240:
> 
>> 238:  * OutOfMemoryError: Requested array size exceeds VM limit
>> 239:  */
>> 240: private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
> 
> Looks like the usage of this field was removed by the 
> https://github.com/openjdk/lanai/commit/03642a01, note that the doc for the 
> "newCapacity" is still mentioned this field.

doc updated

-

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


Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

2021-10-11 Thread John Rose
So the purpose of TWR is to hold an object with a “close-debt”
(debt of a future call to close) and pay it at the end of a block,
sort of like C++ RAII (but also sort of not).

But fluent syntaxes (which I like very much and hope to see
more of in the future!) don’t play well with blocks, so if a
fluent chain (any part of that chain:  It’s multiple objects)
incurs a “close-debt”, it’s hard to jam a TWR block into it.

Hence the current proposal.  I agree with Brian and Paul
that we haven’t examined all the corners of this problem
yet.  And I’d like to poke at the concept of “close-debt” to
help with the examination.

Just for brain storming, I think we could model “close-debt”
outside either fluent API structure or TWR block structure.
Java O-O APIs are the pre-eminent way to model things in
Java, and they work exceedingly well, when used with skill.

AutoCloseable models close-debt of course.  But it has two
weaknesses:  It doesn’t model anything *other* than the
debt, and its (sole) method skates awkwardly around the
issue of checked exceptions.  (It requires an override with
exception type narrowing to be used in polite company.)
AC is more of an integration hook with TWR, rather than
a general-purpose model for close-debt.  Therefore it doesn’t
teach us much about close-debt in a fluent setting.

Surely we can model close-debt better.  Let’s say that an
operation (expression) with close-debt *also* has a return
value and (for grins) *also* has an exception it might throw.
This gets us to an API closer to Optional.  (If you hear the
noise of a monad snuffling around in the dark outside
your tent, you are not wrong.)

interface MustClose_1 {
   T get() throws X;  //or could throw some Y or nothing at all
   void close() throws X;
}

(I wish we had an easier way to associate such an X
with such a T, so that Stream could be more
interoperable with simple Stream.  It’s a pain to
carry around those X arguments.  So I’ll drop X now.)

interface MustClose_2 {
   T get();
   void close() throws Exception;
}

An expression of this type requires (in general) two
operations to finish up:  It must be closed, and the result
(type T) must be gotten.  There’s an issue of coupling between
the two methods; I would say, decouple their order, so that
the “get” call, as with Optional, can be done any time,
and the “close” call can be done in any order relative
to “get”.  Both calls should be idempotent, I think.
But that’s all second-order detail.

A first-order detail is the apparent but incorrect 1-1 relation
between T values and close-debts.  That’s very wrong;
a closable stream on 1,000 values has one close-debt,
not 1,000.  So maybe we need:

interface MustClose_3 {
   S map(Function value);
   void close() throws Exception;
}

That “map” method looks a little like Remi’s apply
method.  Did I mention this design requires skill
(as well as flexibility, with one hand already tied
by checked exceptions)?  I’m at the edge of my own
skill here, but I think there’s good ground to explore
here.

In a fluent setting, a Stream that incurs a close-debt
might be typed (after incurring the debt, perhaps in a
transform) as Stream>, and somehow
all consumers of the MustClose, such as map and
collect operations on the Stream, would correctly
unpack each T from the MC, and then repack
the result into the MC<.> wrapper.

var res = openAFileStream().map(…).collect(…);

Here the first method call returns a Stream with
close-debt mixed into its type.  The map and collect
calls would wire both parts:  The T values flowing
through, and the close-debt.  Who takes responsibility
for paying the close debt?  Maybe an extra call
at the end:  …map(…).collectAndClose(…).
Or maybe the stream “knows” internally that since
its type has a close debt, all of its terminal operations
have to pay off that debt as they collect the payloads.
So it would be automatic, somehow, inside of
collect, forEach, etc.

To make the parts hook up right, you might need
reified generics, or maybe an amended type
MustCloseStream <: Stream>,
like the LongStream <: Stream we dislike.
I’m only proposing as a thought exercise for now.

Maybe the MustCloseStream takes an explicit close
method which produces a regular stream over the
base type.  The explicit close method would release
resources and buffer anything necessary to produce
an in-memory Stream.  You’d want to call it
late in the fluent chain, after filtering and flat-mapping
is done, just before a collect for forEach.

Here’s a streamlined version of MustClose that
I like, which sidesteps the problem of mutual ordering
of two methods:

interface MustClose_4 {
   R getAndClose() throws Exception;
   default void close() throws Exception { getAndClose(); }
}

Here, R is not an element type of a stream, but rather
the final result (maybe Void) of some terminal operation.

Such an interface could interact with syntax, too.  For
example, it might help with TWR expressions (if we
wanted to think about that):

var res 

Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]

2021-10-11 Thread Naoto Sato
On Thu, 9 Sep 2021 08:25:44 GMT, Wu Yan  wrote:

>> Hi,
>> Please help me review the change to enhance getting  time zone ID from 
>> /etc/localtime on linux.
>> 
>> We use `realpath` instead of `readlink` to obtain the link name of 
>> /etc/localtime, because `readlink` can only read the value of a symbolic of 
>> link, not the canonicalized absolute pathname.
>> 
>> For example, the value of /etc/localtime is 
>> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by 
>> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of 
>> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which 
>> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you 
>> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“.
>> 
>> Thanks,
>> wuyan
>
> Wu Yan has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   replace realpath

src/java.base/unix/native/libjava/TimeZone_md.c line 113:

> 111: }
> 112: }
> 113: 

There are a few `*(right + 1)` references in the loops. Is there any 
possibility that it would run over the buffer?

src/java.base/unix/native/libjava/path_util.h line 31:

> 29: int collapsible(char *names);
> 30: void splitNames(char *names, char **ix);
> 31: void joinNames(char *names, int nc, char **ix);

Are these functions, `collapsible`, `splitNames` and `joinNames` have to be 
non-static?

-

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


Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE

2021-10-11 Thread Martin Buchholz
On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov 
 wrote:

> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE

JDK sources should not contain dead unused fields - thanks for fixing.

The change to use newLength in this file should have adjusted the javadoc of 
newCapacity, perhaps simply to refer to ArraysSupport.SOFT_MAX_ARRAY_LENGTH 
instead.

That sounds like a job for Jim Laskey as the author of
commit 03642a01af7123298d6524a98c99a3934d35c11b
Author: Jim Laskey 
Date:   Thu Jun 11 10:08:23 2020 -0300

8230744: Several classes throw OutOfMemoryError without message

Reviewed-by: psandoz, martin, bchristi, rriggs, smarks

If that is fixed (perhaps in a different commit), then this commit is good.

History has shown that capacity growth code is highly errorprone, so it's worth 
writing whitebox tests, as I did in e.g. 

./java/util/concurrent/ConcurrentHashMap/WhiteBox.java
./java/util/ArrayDeque/WhiteBox.java
./java/util/HashMap/WhiteBoxResizeTest.java

-

Marked as reviewed by martin (Reviewer).

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


Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

2021-10-11 Thread Paul Sandoz
Hi Tagir,

Do you mind if we slow down on this and let the idea bake somewhat, captured in 
this thread/issue/PR(draft).

I am always a little wary of compact-only or fluent-only methods, as I find 
them harder to justify. In this case I think there might be something more with 
regards to a pattern/idiom, and making it easier to not forgot to close. But, I 
am not sure we are there yet.

- we have had this “transform” idiom floating around for a while which is like 
your consumeAndClose, but without the try block.
  https://bugs.openjdk.java.net/browse/JDK-8140283
  And such a method was added to String.
There are likely other places where we could consider adding this idiom.

- I think we should explore adding the method you propose on AutoCloseable, 
that likely has more leverage, but we would need to do some careful analysis of 
code to ensure the risk to incompatibly is low.

The idioms “transform” and “transformAndClose” are I think related [*]. If we 
can nail down these I would feel much better about committing to them as 
methods on various classes.

Paul.

[*] If we ever get the notion of Haskell-like type classes in the platform I 
wonder if those would provide the hook, we could add later, that I am looking 
for so that we can corral these idioms.


> On Oct 3, 2021, at 11:51 PM, Tagir F.Valeev  wrote:
> 
> Currently, when the stream holds a resource, it's necessary to wrap it with 
> try-with-resources. This undermines the compact and fluent style of stream 
> API calls. For example, if we want to get the `List` of files inside the 
> directory and timely close the underlying filehandle, we should use something 
> like this:
> 
> 
> List paths;
> try (Stream stream = Files.list(Path.of("/etc"))) {
>paths = stream.toList();
> }
> // use paths
> 
> 
> I suggest to add a new default method to Stream interface named 
> `consumeAndClose`, which allows performing terminal stream operation and 
> closing the stream at the same time. It may look like this:
> 
> 
>default  R consumeAndClose(Function, ? extends R> 
> function) {
>Objects.requireNonNull(function);
>try(this) {
>return function.apply(this);
>}
>}
> 
> 
> Now, it will be possible to get the list of the files in the fluent manner:
> 
> 
> List list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
> 
> -
> 
> Commit messages:
> - Fix tests
> - 8274412: Add a method to Stream API to consume and close the stream without 
> using try-with-resources
> 
> Changes: https://git.openjdk.java.net/jdk/pull/5796/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5796=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8274412
>  Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/5796.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5796/head:pull/5796
> 
> PR: https://git.openjdk.java.net/jdk/pull/5796



Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v2]

2021-10-11 Thread Ravi Reddy
On Mon, 11 Oct 2021 13:42:35 GMT, Ravi Reddy  wrote:

>> Hi all,
>> 
>> Please review this fix for Infinite loop in ZipOutputStream.close().
>> The main issue here is when ever there is an exception during close 
>> operations on GZip we are not setting the deflator to a finished state which 
>> is leading to an infinite loop when we try writing on the same GZip 
>> instance( since we use while(!def.finished()) inside the write operation).
>> 
>> Thanks,
>> Ravi
>
> Ravi Reddy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8193682 : Infinite loop in ZipOutputStream.close()

I have updated the review with the new fix.

Instead of throwing an exception in close() method , Now when we get an 
exception during write inside deflate() , we will close the stream and throw 
the exception.

Updated the test case in TestNG format.

-

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


Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v2]

2021-10-11 Thread Ravi Reddy
> Hi all,
> 
> Please review this fix for Infinite loop in ZipOutputStream.close().
> The main issue here is when ever there is an exception during close 
> operations on GZip we are not setting the deflator to a finished state which 
> is leading to an infinite loop when we try writing on the same GZip instance( 
> since we use while(!def.finished()) inside the write operation).
> 
> Thanks,
> Ravi

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

  8193682 : Infinite loop in ZipOutputStream.close()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5522/files
  - new: https://git.openjdk.java.net/jdk/pull/5522/files/5072b6c1..f6a678ed

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

  Stats: 96 lines in 2 files changed: 48 ins; 28 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5522.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5522/head:pull/5522

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


Re: RFR: 8177814: jdk/editpad is not in jdk TEST.groups

2021-10-11 Thread Aleksey Shipilev
On Thu, 23 Sep 2021 08:54:48 GMT, Aleksey Shipilev  wrote:

> @prrace notices this here: 
> https://github.com/openjdk/jdk/pull/5544#issuecomment-925396869. And I think 
> it is the already open issue that this patch is fixing. While the original 
> patch added the test in `jdk_other`, Phil suggests it to be added to 
> `jdk_desktop`.
> 
> Additional testing:
>  - [x] `jdk_editpad` is passing

Anyhow, how do you want to proceed with this PR?

-

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


Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]

2021-10-11 Thread Raffaello Giulietti
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti 
 wrote:

>> Hello,
>> 
>> here's a PR for a patch submitted on March 2020 
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was 
>> a thing.
>> 
>> The patch has been edited to adhere to OpenJDK code conventions about 
>> multi-line (block) comments. Nothing in the code proper has changed, except 
>> for the addition of redundant but clarifying parentheses in some expressions.
>> 
>> 
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   4511638: Double.toString(double) sometimes produces incorrect results

Hi Guy,

while implementing the additional test recommended in your point (2), it 
occurred to me that the numbers of the form y 10^n, y in D_k (k = 1, 2, 3, 4) 
end up being of the form y' 10^n', where y' = y / 10^k, n' = n + k, plus the 2 
* Y values around these. Such numbers do not seem to show any special structure 
worth a dedicated test, so I'm wondering if you mean something else instead.

Perhaps you mean y to have at most 4 digits, i.e., 0 <= y < 10^4?


Greetings
Raffaello

P.S. The test recommended in point (1) pass successfully.

-

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