RFR: JDK-8311113: Remove invalid pointer cast and clean up setLabel() in awt_MenuItem.cpp [v3]
Alexey Ivanov
aivanov at openjdk.org
Thu Aug 31 12:53:03 UTC 2023
On Fri, 25 Aug 2023 19:18:45 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:
>> In awt_MenuItem.cpp (712,22): ` mii.dwTypeData = (LPTSTR)(*sb)` produces invalid pointer cast warning when complied on clang and moreover this is a no-op.
>>
>> `mii.dwTypeData` is used only when **MIIM_STRING** flag is set in the fMask (as per [Docs](https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-menuiteminfoa)), which is not the case in JDK [Ln#705](https://github.com/openjdk/jdk/blob/e56d3bc2dab3d32453b6eda66e8434953c436084/src/java.desktop/windows/native/libawt/windows/awt_MenuItem.cpp#L706). Hence the assignment ` mii.dwTypeData = (LPTSTR)(*sb)` is not required and so is the label parameter. Additionally necessary cleanup is done at the following places -
>>
>> - WMenuItemPeer.java - to the native function call
>> - awt_MenuItem.cpp - `WMenuItemPeer__1setLabel() ,_SetLabel(), SetLabel()`
>> - awt_MenuItem.h
>>
>> Added a test which checks setLabel() functionality on Menu, MenuItem and PopupMenu.
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
>
> removed robot.waitForIdle() calls
The changes look good to me.
test/jdk/java/awt/MenuItem/SetLabelTest.java line 54:
> 52: private static PopupMenu pm;
> 53: private static Point frameLoc;
> 54: private static StringBuffer errorLog = new StringBuffer();
`errorLog` could be `final`.
test/jdk/java/awt/MenuItem/SetLabelTest.java line 127:
> 125: }
> 126:
> 127: private static void checkMenu() throws Exception {
Suggestion:
private static void checkMenu() throws IOException {
To be consistent with `checkPopupMenu`?
-------------
Marked as reviewed by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15276#pullrequestreview-1604677389
PR Review Comment: https://git.openjdk.org/jdk/pull/15276#discussion_r1311580105
PR Review Comment: https://git.openjdk.org/jdk/pull/15276#discussion_r1311577752
More information about the client-libs-dev
mailing list