Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-26 Thread Hamlin Li
On Mon, 26 Apr 2021 14:55:57 GMT, Lance Andersen  wrote:

> Hi Hamlin,
> 
> The change looks fine. Please add the noreg-trivial change to the issue given 
> there is not a test update for this so that you do not get pinged by a bot

Hi Lance, Thanks for reminding.

Thanks @LanceAndersen @naotoj @vyommani for your review.

-

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


Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-26 Thread Naoto Sato
On Mon, 26 Apr 2021 02:36:54 GMT, Hamlin Li  wrote:

>> code like below will create Deflater before null check, although it's not a 
>> real mem leak, but it's better to do null check before new Deflater.
>> 
>> try {
>> DeflaterOutputStream dos = new DeflaterOutputStream(null);
>> } catch (NullPointerException e) {
>> passed = true;
>> }
>> Similar issues exist in several other classes.
>
> Hamlin Li has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update copyrights.

Marked as reviewed by naoto (Reviewer).

-

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


Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-26 Thread Lance Andersen
On Mon, 26 Apr 2021 02:36:54 GMT, Hamlin Li  wrote:

>> code like below will create Deflater before null check, although it's not a 
>> real mem leak, but it's better to do null check before new Deflater.
>> 
>> try {
>> DeflaterOutputStream dos = new DeflaterOutputStream(null);
>> } catch (NullPointerException e) {
>> passed = true;
>> }
>> Similar issues exist in several other classes.
>
> Hamlin Li has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update copyrights.

Hi Hamlin,

The change looks fine.  Please add the noreg-trivial change to the issue given 
there is not a test update for this so that you do not get pinged by a bot

-

Marked as reviewed by lancea (Reviewer).

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


Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-26 Thread Hamlin Li
On Mon, 26 Apr 2021 02:36:54 GMT, Hamlin Li  wrote:

>> code like below will create Deflater before null check, although it's not a 
>> real mem leak, but it's better to do null check before new Deflater.
>> 
>> try {
>> DeflaterOutputStream dos = new DeflaterOutputStream(null);
>> } catch (NullPointerException e) {
>> passed = true;
>> }
>> Similar issues exist in several other classes.
>
> Hamlin Li has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update copyrights.

It's changed by another patch sometimes ago, but I missed to update the 
copyright.

-

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


Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-25 Thread Vyom Tewari
On Mon, 26 Apr 2021 02:36:54 GMT, Hamlin Li  wrote:

>> code like below will create Deflater before null check, although it's not a 
>> real mem leak, but it's better to do null check before new Deflater.
>> 
>> try {
>> DeflaterOutputStream dos = new DeflaterOutputStream(null);
>> } catch (NullPointerException e) {
>> passed = true;
>> }
>> Similar issues exist in several other classes.
>
> Hamlin Li has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update copyrights.

Changes looks OK to me, did you change  "DataOutputStreamTest.java" as well. I 
can see only copyright changes ?.

-

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


Re: RFR: JDK-8265496: improve null check in DeflaterOutputStream/InflaterInputStream [v2]

2021-04-25 Thread Hamlin Li
> code like below will create Deflater before null check, although it's not a 
> real mem leak, but it's better to do null check before new Deflater.
> 
> try {
> DeflaterOutputStream dos = new DeflaterOutputStream(null);
> } catch (NullPointerException e) {
> passed = true;
> }
> Similar issues exist in several other classes.

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

  update copyrights.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3681/files
  - new: https://git.openjdk.java.net/jdk/pull/3681/files/25a88c61..38c45b62

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

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

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