Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Thanks for pushing changeset 865b5ca81009. Raffaello On 2020-07-21 19:18, Lance Andersen wrote: Hi Roger, I will plan to push tomorrow morning pending any last minute reviews Best Lance On Jul 21, 2020, at 9:57 AM, Roger Riggs > wrote: Hi Rafaello, Lance,

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Roger, I will plan to push tomorrow morning pending any last minute reviews Best Lance > On Jul 21, 2020, at 9:57 AM, Roger Riggs wrote: > > Hi Rafaello, Lance, > > That looks good to me. > > Thanks, Roger > > On 7/19/20 2:31 PM, Lance Andersen wrote: >> Hi Raffaello, >> >> I think

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Rafaello, Lance, That looks good to me. Thanks, Roger On 7/19/20 2:31 PM, Lance Andersen wrote: Hi Raffaello, I think the changes look reasonable. I have created a webrev, http://cr.openjdk.java.net/~lancea/8222187/webrev.00/, for others to review as it is a bit easier than the patch.

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Raffaello, I think the changes look reasonable. I have created a webrev, http://cr.openjdk.java.net/~lancea/8222187/webrev.00/ , for others to review as it is a bit easier than the patch. I have also run the JCK tests in this area as

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Roger, On 2020-07-15 22:21, Roger Riggs wrote: Hi Raffaello, Base64.java: 2: Please update 2nd copyright year to 2020 I was unsure what to do here, thanks for guidance. I also removed the seemingly useless import at former L33. leftovers(): 996: "&" the shortcutting && is more

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Raffaello, Base64.java: 2: Please update 2nd copyright year to 2020 leftovers(): 996: "&" the shortcutting && is more typical in a condition. 997: the -= is buried in an expression and a reader might miss it. 1053:  "can be reallocated to other vars":  not a useful comment, reflecting on

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Roger, here's the latest version of the patch. I did: * Withdraw the simplification in encodedOutLength(), as it is indeed out of scope for this bug fix. * Restore field names in DecInputStream except for nextin (now wpos) and nextout (now rpos) because they have slightly different

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Raffaello, Thank you for your taking up this issue. When working on a fix such as this, it is best to keep the changes narrowly focused to the issue at hand and not attempt to address additional issues within the same fix (though this can be tempting). To move forward, I would suggest

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hello Roger, I would of course retract the changes in encodedOutLength() because, as you observe, it is out of scope w.r.t. the bug. I would also retract the other minor changes you mention and clean up the appearance of the comments to adhere to community shared practices. Thus, if you

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Rafaello, Since you have expanded the scope of the fix, you will need to update the bug to include the other changes you are making.  Or create a new issue for the additional work. Personal preference in style has very little place when maintaining existing code. The style should match

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Roger, the structure of the original design is preserved, the details change with the intent of having code that is easier to reason about. The main loop has many exit points in the original code, as well as in the new code. For this reason, I prefer not to express it as a while or as a

Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi Raffaello, I'm still not sure it needed this much of a re-write to fix the bug; It will take some time to look at the changes. Regardless, OpenJDK conventions call for following the style of the existing code. Your new comments follow neither the existing convention to use "//..." comments

RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hello, after Roger's notes, which escaped my attention before, I've withdrawn all the changes but for DecInputStream, *except* that I couldn't resist to simplify the maths in encodedOutLength(), while still using xxxExact() arithmetic. Sorry for the confusion Raffaello Hi Raffaello,

RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end

Hi, with respect to [1], this version of the patch includes performance enhancements for the case of small buffers, in particular for the implementation of the no-arg read(), which makes use of a one-byte array. I also expanded the comments. As I don't have a webrev repo, I beg the