8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification #26104

bplb · 2025-07-02T20:24:19Z

Modify CharBuffer.getChars to respect the class level specification that the provided buffer offset srcBegin is relative to the buffer's current position.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8361308 to be approved

Issues

JDK-8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification (Bug - P2)
JDK-8361308: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification (CSR)

Reviewers

Chen Liang (@liach - Reviewer) Review applies to e9abd20f
Alan Bateman (@AlanBateman - Reviewer)

Reviewers without OpenJDK IDs

@bokken (no known openjdk.org user name / role) Review applies to e9abd20f

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26104/head:pull/26104
$ git checkout pull/26104

Update a local copy of the PR:
$ git checkout pull/26104
$ git pull https://git.openjdk.org/jdk.git pull/26104/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26104

View PR using the GUI difftool:
$ git pr show -t 26104

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26104.diff

Using Webrev

Link to Webrev Comment

…isting specification

bplb · 2025-07-02T20:24:33Z

/csr

bridgekeeper · 2025-07-02T20:25:09Z

👋 Welcome back bpb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-07-02T20:25:51Z

@bplb This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification

Reviewed-by: alanb, liach

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 14 new commits pushed to the master branch:

25ed36f: 8359493: Refactor how aggregated mandatory warnings are handled in the compiler
a2315dd: 8357739: [jittester] disable the hashCode method
66836d4: 8361292: Rename ModuleEntry::module() to module_oop()
... and 11 more: https://git.openjdk.org/jdk/compare/ea86a20e6d74baea54df32415d9096d3b7bba1d7...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-07-02T20:26:22Z

@bplb has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@bplb please create a CSR request for issue JDK-8361299 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

openjdk · 2025-07-02T20:26:47Z

@bplb The following label will be automatically applied to this pull request:

nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-07-02T20:30:03Z

Webrevs

bplb · 2025-07-02T22:33:50Z

jdk_core tests passed on Linux in the CI.

mlbridge · 2025-07-02T22:54:32Z

Mailing list message from Brian Burkhalter on nio-dev:

On Jul 2, 2025, at 2:49?PM, Brett Okken <duke at openjdk.org> wrote:

Marked as reviewed by bokken at github.com<mailto:bokken at github.com> (no known OpenJDK username).

Thanks for the review.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/nio-dev/attachments/20250702/fb756742/attachment-0001.htm>

AlanBateman · 2025-07-03T09:40:07Z

src/java.base/share/classes/java/nio/X-Buffer.java.template

+     * {@code position() + srcBegin} in this buffer and at offset
+     * {@code dstBegin} in the array. The position of this buffer is unchanged.
+     *
+     * <p> An invocation of this method behaves exactly the same was as the


I think we can use an implSpec for this and keep "This method is equivalent to".

I think we can use an implSpec for this and keep "This method is equivalent to".

I was following the existing convention elsewhere in the file.

I don't know why JDK-8343110 put the implSpec after the param/return/throws tags so happy it has been fixed. Maybe at some point we can use implSpec to these methods.

AlanBateman · 2025-07-03T09:51:21Z

test/jdk/java/nio/Buffer/GetChars.java

@@ -71,6 +71,19 @@ public void testSrcBeginIsNegative() {
                () -> CB.getChars(-1, 3, new char[4], 0));
    }

+    @Test
+    public void testSrcBeginIsNegationOfPosition() {


I skimmed through GetChars and I see it only tests a CB created with CharBuffer.wrap, only testGetChars tests all char buffers. All the test methods should be tested all char buffer implementations. So maybe we can create a follow-up issue to improve this test (and probably migrate it to a JUnit test too).

So maybe we can create a follow-up issue to improve this test (and probably migrate it to a JUnit test too).

Agreed. I also noticed it is TestNG now.

Okay, let's create a follow-up issue for that.

8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-ex…

5bbc34d

…isting specification

openjdk bot added rfr Pull request is ready for review csr Pull request needs approved CSR before integration labels Jul 2, 2025

openjdk bot added the nio [email protected] label Jul 2, 2025

8361299: Fix a type; add a sentence

e9abd20

bokken approved these changes Jul 2, 2025

View reviewed changes

liach approved these changes Jul 2, 2025

View reviewed changes

8361299: Ensure [srcBegin,srcEnd) is contained in [0,limit()-position())

a98e8dd

AlanBateman reviewed Jul 3, 2025

View reviewed changes

AlanBateman approved these changes Jul 3, 2025

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels Jul 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification #26104

8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification #26104

bplb commented Jul 2, 2025 •

edited by openjdk bot

Loading

Uh oh!

bplb commented Jul 2, 2025

Uh oh!

bridgekeeper bot commented Jul 2, 2025

Uh oh!

openjdk bot commented Jul 2, 2025 •

edited

Loading

Uh oh!

openjdk bot commented Jul 2, 2025

Uh oh!

openjdk bot commented Jul 2, 2025

Uh oh!

mlbridge bot commented Jul 2, 2025 •

edited

Loading

Uh oh!

bplb commented Jul 2, 2025

Uh oh!

mlbridge bot commented Jul 2, 2025

Uh oh!

AlanBateman Jul 3, 2025

Uh oh!

bplb Jul 3, 2025

Uh oh!

AlanBateman Jul 3, 2025

Uh oh!

AlanBateman Jul 3, 2025

Uh oh!

bplb Jul 3, 2025

Uh oh!

AlanBateman Jul 3, 2025

Uh oh!

Uh oh!

8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification #26104

Are you sure you want to change the base?

8361299: (bf) CharBuffer.getChars(int,int,char[],int) violates pre-existing specification #26104

Conversation

bplb commented Jul 2, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewers without OpenJDK IDs

Reviewing

Uh oh!

bplb commented Jul 2, 2025

Uh oh!

bridgekeeper bot commented Jul 2, 2025

Uh oh!

openjdk bot commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jul 2, 2025

Uh oh!

openjdk bot commented Jul 2, 2025

Uh oh!

mlbridge bot commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

bplb commented Jul 2, 2025

Uh oh!

mlbridge bot commented Jul 2, 2025

Uh oh!

AlanBateman Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

bplb Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

AlanBateman Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

AlanBateman Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

bplb Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

AlanBateman Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bplb commented Jul 2, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jul 2, 2025 •

edited

Loading

mlbridge bot commented Jul 2, 2025 •

edited

Loading