Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-10 Thread Markus KARG
On Mon, 5 Sep 2022 18:33:43 GMT, Alan Bateman wrote: >> Markus KARG has updated the pull request incrementally with one additional >> commit since the last revision: >> >> HexPrinter::transferTo > > It looks like this patch is against a repository that hasn't been sync'ed up > since late

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-07 Thread Alan Bateman
On Tue, 6 Sep 2022 16:29:39 GMT, Alan Bateman wrote: >> BIS dates from JDK 1.0 and it's not hard to find examples that extend it. >> The HexPrinter test reminds us that it has been possible for 25+ years to >> override the read methods and snoop on all bytes that are read. Adding an >>

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Tue, 6 Sep 2022 17:11:39 GMT, Daniel Fuchs wrote: >> Thank you all for your kind help and feedback. >> >> @AlanBateman So is now the time to switch this PR from Draft to Ready? > > The source changes LGTM. I suppose you should now revert the changes above > (in `HexPrinter.java`), otherwise

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Daniel Fuchs
On Tue, 6 Sep 2022 17:01:09 GMT, Markus KARG wrote: >>> @AlanBateman Async close leaves BIS in an invalid state. The JavaDocs say ` >>> The behavior for the case where the input and/or output stream is >>> asynchronously closed, or the thread interrupted during the transfer, is >>> highly

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Tue, 6 Sep 2022 16:29:39 GMT, Alan Bateman wrote: >> BIS dates from JDK 1.0 and it's not hard to find examples that extend it. >> The HexPrinter test reminds us that it has been possible for 25+ years to >> override the read methods and snoop on all bytes that are read. Adding an >>

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Alan Bateman
On Tue, 6 Sep 2022 12:14:37 GMT, Alan Bateman wrote: >> With the changes you proposed a CSR will definitely be needed. > > BIS dates from JDK 1.0 and it's not hard to find examples that extend it. The > HexPrinter test reminds us that it has been possible for 25+ years to > override the read

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Daniel Fuchs
On Tue, 6 Sep 2022 13:00:23 GMT, Markus KARG wrote: >> Rebased and fixed locking, using your proposed code now. >> >> @AlanBateman Async close leaves BIS in an invalid state. The JavaDocs say ` >> The behavior for the case where the inputand/or output stream is >> asynchronously closed, or

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Tue, 6 Sep 2022 12:32:46 GMT, Markus KARG wrote: >> BIS dates from JDK 1.0 and it's not hard to find examples that extend it. >> The HexPrinter test reminds us that it has been possible for 25+ years to >> override the read methods and snoop on all bytes that are read. Adding an >>

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Tue, 6 Sep 2022 12:56:46 GMT, Markus KARG wrote: >> Thank you for your decision about this issue, so instead of fixing the >> existing bugs I will implement as you outlined. >> >> BTW, locking and rebasing is on the way. > > Rebased and fixed locking, using your proposed code now. > >

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Tue, 6 Sep 2022 12:14:37 GMT, Alan Bateman wrote: >> With the changes you proposed a CSR will definitely be needed. > > BIS dates from JDK 1.0 and it's not hard to find examples that extend it. The > HexPrinter test reminds us that it has been possible for 25+ years to > override the read

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Alan Bateman
On Tue, 6 Sep 2022 12:10:00 GMT, Daniel Fuchs wrote: >>> It's a well known behavior that overriding the`read(...)` method is >>> sufficient to implement subclass behavior. This will no longer be the case >>> if `transferTo(...)` no longer calls `this.read(...)` as it used to. There >>> are

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Daniel Fuchs
On Tue, 6 Sep 2022 11:26:18 GMT, Markus KARG wrote: >> It's a well known behavior that overriding the`read(...)` method is >> sufficient to implement subclass behavior. This will no longer be the case >> if `transferTo(...)` no longer calls `this.read(...)` as it used to. There >> are many

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Tue, 6 Sep 2022 09:49:16 GMT, Daniel Fuchs wrote: > It's a well known behavior that overriding the`read(...)` method is > sufficient to implement subclass behavior. This will no longer be the case if > `transferTo(...)` no longer calls `this.read(...)` as it used to. There are > many

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Daniel Fuchs
On Tue, 6 Sep 2022 07:55:55 GMT, Markus KARG wrote: >> test/lib/jdk/test/lib/hexdump/HexPrinter.java line 1194: >> >>> 1192: byteOffset += size; >>> 1193: return size; >>> 1194: } >> >> This is an indication that overriding `transferTo()` in >>

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-06 Thread Markus KARG
On Mon, 5 Sep 2022 20:21:01 GMT, Daniel Fuchs wrote: >> Markus KARG has updated the pull request incrementally with one additional >> commit since the last revision: >> >> HexPrinter::transferTo > > test/lib/jdk/test/lib/hexdump/HexPrinter.java line 1194: > >> 1192: byteOffset

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-05 Thread Daniel Fuchs
On Mon, 5 Sep 2022 18:05:04 GMT, Markus KARG wrote: >> Implementation of JDK-8279283 > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > HexPrinter::transferTo test/lib/jdk/test/lib/hexdump/HexPrinter.java line 1194: > 1192:

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-05 Thread Alan Bateman
On Mon, 5 Sep 2022 18:05:04 GMT, Markus KARG wrote: >> Implementation of JDK-8279283 > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > HexPrinter::transferTo It looks like this patch is against a repository that hasn't been

Re: RFR: 8279283 - BufferedInputStream should override transferTo [v7]

2022-09-05 Thread Markus KARG
> Implementation of JDK-8279283 Markus KARG has updated the pull request incrementally with one additional commit since the last revision: HexPrinter::transferTo - Changes: - all: https://git.openjdk.org/jdk/pull/6935/files - new: