On Wed, 3 Jul 2024 13:34:38 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> In [JDK-8302671](https://bugs.openjdk.org/browse/JDK-8302671) I fixed a >> memmove decay bug by rewriting a sizeof on an array to an explicit size of >> 256, but this is a bit of a band aid fix. It's come to my attention that in >> C++, one can pass an array by reference, which causes sizeof to work >> correctly on an array and has the added bonus of enforcing an array of that >> size on the arguments passed to that method. I've reverted my change from >> 8302671 and instead explicitly made kstate an array reference so that sizeof >> works on the array as expected, and that the array size can be explicitly >> set in the array brackets >> >> Verification: https://godbolt.org/z/Ezj76eWWY and GitHub Actions > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - Formatting sizeof awt_Component.cpp > - Formatting awt_Component.cpp > - Merge branch 'openjdk:master' into patch-10 > - Update src/java.desktop/windows/native/libawt/windows/awt_Component.cpp > > Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> > - 8334599 Still looks good, except for the minor formatting comments. src/java.desktop/windows/native/libawt/windows/awt_Component.cpp line 3365: > 3363: } > 3364: static void > 3365: resetKbdState(BYTE (&kstate) [AwtToolkit::KB_STATE_SIZE]) { I don't know what is most used syntax for this type. I'd rather keep them together without a space between `()` and `[]`. src/java.desktop/windows/native/libawt/windows/awt_Component.cpp line 3368: > 3366: BYTE tmpState[AwtToolkit::KB_STATE_SIZE]; > 3367: WCHAR wc[2]; > 3368: memmove(tmpState, kstate, sizeof (kstate)); Suggestion: memmove(tmpState, kstate, sizeof(kstate)); There's usually no space after `sizeof`. ------------- Marked as reviewed by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19798#pullrequestreview-2158844075 PR Review Comment: https://git.openjdk.org/jdk/pull/19798#discussion_r1665680146 PR Review Comment: https://git.openjdk.org/jdk/pull/19798#discussion_r1665657251
