Re: RFR 8139206: Add InputStream readNBytes(int len)

I have moved the CSR [1] back to Draft and updated it according to the content of webrev.03. If there are no more comments by tomorrow I will move it once again to Finalized. After that, once the CSR has been re-approved, then if there are no further comments on the changes I will go ahead and

Re: RFR 8139206: Add InputStream readNBytes(int len)

+1 On 1/24/2018 2:50 PM, Brian Burkhalter wrote: On Jan 23, 2018, at 4:50 PM, Brian Burkhalter > wrote: On Jan 23, 2018, at 1:19 AM, Weijun Wang > wrote: + *

Re: RFR 8139206: Add InputStream readNBytes(int len)

On Jan 23, 2018, at 4:50 PM, Brian Burkhalter wrote: > On Jan 23, 2018, at 1:19 AM, Weijun Wang wrote: > >> + * Therefore, the method may be safely called with very large values of >> + * {@code len} provided sufficient memory is

Re: RFR 8139206: Add InputStream readNBytes(int len)

On Jan 23, 2018, at 1:19 AM, Weijun Wang wrote: > + * Therefore, the method may be safely called with very large values of > + * {@code len} provided sufficient memory is available. > > What does "sufficient memory" mean? For len, or the number of available

Re: RFR 8139206: Add InputStream readNBytes(int len)

+ * Therefore, the method may be safely called with very large values of + * {@code len} provided sufficient memory is available. What does "sufficient memory" mean? For len, or the number of available bytes? --Max > On Jan 23, 2018, at 4:49 AM, Brian Burkhalter

Re: RFR 8139206: Add InputStream readNBytes(int len)

On 22/01/2018 21:27, Brian Burkhalter wrote: I have updated this verbiage in place in webrev.02 and webrev.01-02. Updated wording looks okay to me. -Alan

Re: RFR 8139206: Add InputStream readNBytes(int len)

On Jan 22, 2018, at 1:22 PM, Alan Bateman wrote: > On 22/01/2018 21:12, Brian Burkhalter wrote: >> >> I can change it. The reason I used that verbiage was to match that of >> read(byte[]): >> >> "The read(b) method for class InputStream has the same effect as: >>

Re: RFR 8139206: Add InputStream readNBytes(int len)

On 22/01/2018 21:12, Brian Burkhalter wrote: I can change it. The reason I used that verbiage was to match that of read(byte[]):  "The |read(b)| method for class |InputStream| has the same effect as:  read(b, 0, b.length)" True but the equivalent in readAllBytes won't guarantee that

Re: RFR 8139206: Add InputStream readNBytes(int len)

On Jan 22, 2018, at 1:09 PM, Alan Bateman wrote: > On 22/01/2018 20:49, Brian Burkhalter wrote: >> All of the comments included below are addressed in [1]. The difference >> versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, >> updated, and

Re: RFR 8139206: Add InputStream readNBytes(int len)

On 22/01/2018 20:49, Brian Burkhalter wrote: All of the comments included below are addressed in [1]. The difference versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, updated, and re-finalized but I will hold off on that until there is a final consensus on the verbiage.

Re: RFR 8139206: Add InputStream readNBytes(int len)

All of the comments included below are addressed in [1]. The difference versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, updated, and re-finalized but I will hold off on that until there is a final consensus on the verbiage. Thanks, Brian [1]

Re: RFR 8139206: Add InputStream readNBytes(int len)

Certainly some verbiage like that could be added. Going back to last month in the discussion about improving the performance of readAllBytes() I calculated the exact number of bytes allocated [1]. For the initial implementation in that change this was B + L for L <= B N = B +

Re: RFR 8139206: Add InputStream readNBytes(int len)

The spec of the new method doesn't give me enough information to determine whether it is safe to call it when the value of the length argument is much larger than the number of bytes I expect to actually read. This use case comes up frequently in security libraries, because we have to handle

Re: RFR 8139206: Add InputStream readNBytes(int len)

On Jan 22, 2018, at 7:05 AM, Weijun Wang wrote: >> Both methods that throw and not throw have been proposed. But adding two >> methods >> seems like to too much clutter in the API and the methods appear too similar. > > Sorry I wasn't aware of earlier discussions on

Re: RFR 8139206: Add InputStream readNBytes(int len)

> On Jan 22, 2018, at 10:16 PM, Roger Riggs wrote: > > Hi Max, > > Both methods that throw and not throw have been proposed. But adding two > methods > seems like to too much clutter in the API and the methods appear too similar. Sorry I wasn't aware of earlier

Re: RFR 8139206: Add InputStream readNBytes(int len)

> On Jan 22, 2018, at 5:01 PM, Weijun Wang wrote: > > I wonder when readNBytes(n) will be useful if the return value has less than > n bytes. I mean I have seen protocols saying reading rest of the content, or reading N bytes, but never reading at most N bytes. I

Re: RFR 8139206: Add InputStream readNBytes(int len)

On 22/01/2018 08:44, Peter Levart wrote: The delegation to public method (readAllBytes -> readNBytes) should then be documented so that subclasses know that overriding readNBytes, if needed, is sufficient. It doesn't delegate in the latest version of the patch so the documentation is right.

Re: RFR 8139206: Add InputStream readNBytes(int len)

I wonder when readNBytes(n) will be useful if the return value has less than n bytes. > On Jan 22, 2018, at 4:52 PM, Alan Bateman wrote: > > > > On 17/01/2018 16:24, Brian Burkhalter wrote: >> : >> >> A negative value of ‘len’ will now cause an

Re: RFR 8139206: Add InputStream readNBytes(int len)

On 17/01/2018 16:24, Brian Burkhalter wrote: : A negative value of ‘len’ will now cause an IllegalArgumentException instead of an IndexOutOfBoundsException. Also some verbiage has been improved. http://cr.openjdk.java.net/~bpb/8139206/webrev.01/ The updated version looks good. I just wonde

Re: RFR 8139206: Add InputStream readNBytes(int len)

Hi, On 01/19/2018 08:49 PM, Roger Riggs wrote: Hi Brian, Looks good, A pre-existing typo:     line 67 "{@code skip()}" should be "{@code skip(*long*)}". Since the public readNBytes suffices for readAllBytes, I would rename the private readAtMostNBytes to readNBytes and avoid the

Re: RFR 8139206: Add InputStream readNBytes(int len)

Hi Brian, Looks good, A pre-existing typo:     line 67 "{@code skip()}" should be "{@code skip(*long*)}". Since the public readNBytes suffices for readAllBytes, I would rename the private readAtMostNBytes to readNBytes and avoid the duplication of javadoc. Keeping the existing readAllBytes

Re: RFR 8139206: Add InputStream readNBytes(int len)

The proposed change has been modified to replace the two methods byte[] InputStream.readAllBytes(int) // reads at most ‘len’ bytes byte[] InputStream.readNBytes(int) // reads exactly ‘len’ bytes or throws IOException with a single method byte[] InputStream.readNBytes(int) // reads at most