Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Hello Lance, On 08/07/19 11:16 PM, Lance Andersen wrote: > Hi Jaikiran, > > Thank you for your efforts here. > >> On Jul 6, 2019, at 9:59 AM, Jaikiran Pai > > wrote: >> >>> >>>  - Idempotency >>> >>> Yes, it looks to me like Inflater.end() and Deflater.end() >>>

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Hi Jaikiran, Thank you for your efforts here. > On Jul 6, 2019, at 9:59 AM, Jaikiran Pai wrote: > >> >> - Idempotency >> >> Yes, it looks to me like Inflater.end() and Deflater.end() >> implementations are idempotent. As you point out, overrides by >> subclasses might not be. We should be

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Hello Lance, On 03/07/19 11:27 PM, Lance Andersen wrote: >  Just a thought below to consider or ignore ;-)  >> On Jul 2, 2019, at 11:27 PM, Stuart Marks > > wrote: >> >> Hi Jaikiran, >> >> OK, good analysis. Rather a lot of issues for what seems like a >> simple

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Hello Stuart, On 03/07/19 8:57 AM, Stuart Marks wrote: > Hi Jaikiran, > > OK, good analysis. Rather a lot of issues for what seems like a simple > patch, eh? Indeed, but I kind of expected that :) > >  - Idempotency > > Yes, it looks to me like Inflater.end() and Deflater.end() >

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Just a thought below to consider or ignore ;-) > On Jul 2, 2019, at 11:27 PM, Stuart Marks wrote: > > Hi Jaikiran, > > OK, good analysis. Rather a lot of issues for what seems like a simple patch, > eh? > > - Idempotency > > Yes, it looks to me like Inflater.end() and Deflater.end()

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Hi Jaikiran, OK, good analysis. Rather a lot of issues for what seems like a simple patch, eh? - Idempotency Yes, it looks to me like Inflater.end() and Deflater.end() implementations are idempotent. As you point out, overrides by subclasses might not be. We should be clear when we're

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

On 29/06/19 4:31 PM, Jaikiran Pai wrote: > Hello Stuart, > > Thank you for the detailed response. Comments inline. > > On 28/06/19 2:48 AM, Stuart Marks wrote: >> On 6/26/19 9:28 PM, Jaikiran Pai wrote: >>> I am looking to contribute a patch for the enhancement noted in >>>

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Hello Stuart, Thank you for the detailed response. Comments inline. On 28/06/19 2:48 AM, Stuart Marks wrote: > On 6/26/19 9:28 PM, Jaikiran Pai wrote: >> I am looking to contribute a patch for the enhancement noted in >> https://bugs.openjdk.java.net/browse/JDK-8225763. The change itself >>

Re: Inputs on patch for JDK-8225763?

Hello Alan, On 27/06/19 4:22 PM, Alan Bateman wrote: > On 27/06/2019 11:16, Remi Forax wrote: >> Is a boolean isEnded that records if end() was already called is not >> enough ? >> > I'm sure Jaikiran will create tests that cover the different > combinations of overriding end and close. It might

Re: Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

On 6/26/19 9:28 PM, Jaikiran Pai wrote: I am looking to contribute a patch for the enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8225763. The change itself looks relatively straightforward to me and here's what I plan to do: 1. Have both java.util.zip.Inflater and

Re: Inputs on patch for JDK-8225763?

On 27/06/2019 11:16, Remi Forax wrote: Is a boolean isEnded that records if end() was already called is not enough ? I'm sure Jaikiran will create tests that cover the different combinations of overriding end and close. It might enough for close to check if zsRef.address is 0. -Alan.

Re: Inputs on patch for JDK-8225763?

Is a boolean isEnded that records if end() was already called is not enough ? Rémi - Mail original - > De: "Alan Bateman" > À: "Jaikiran Pai" , "core-libs-dev" > > Envoyé: Jeudi 27 Juin 2019 09:13:41 > Objet: Re: Inputs on patch for JDK-

Re: Inputs on patch for JDK-8225763?

On 27/06/2019 05:28, Jaikiran Pai wrote: : 2. Have the close() method call the end() method One subtle point is that end() is not idempotent. The close() method defined by AutoCloseable is not required be to either but we should try hard to specify Inflater::close to be idempotent. It might

Inputs on patch for JDK-8225763?

I am looking to contribute a patch for the enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8225763. The change itself looks relatively straightforward to me and here's what I plan to do: 1. Have both java.util.zip.Inflater and java.util.zip.Deflater start implementing the