On Fri, 25 Aug 2023 19:18:45 GMT, Harshitha Onkar <[email protected]> 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 > I would also add that the pointer saved to `mii.dwTypeData` becomes invalid > as soon as `m->SetLabel(labelPtr)` returns because the code in `_SetLabel` > releases the pointer `labelPtr`. > > Essentially, this was the code flow in `_SetLabel`: > > ```c++ > LPCTSTR labelPtr = JNU_GetStringPlatformChars(env, label, 0); > m->SetLabel(labelPtr); > JNU_ReleaseStringPlatformChars(env, label, labelPtr); > ``` > > If any code had dereferenced the pointer stored for a menu item in > `dwTypeData`, the process would've crashed with access violation, or it > could've led to a memory corruption. I don't think that's relevant. "mii" is stack allocated and the code does ::InsertMenuItem(hMenu, idx, TRUE, &mii); and this pattern occurs in other places too. So I conclude that - although it isn't documented SFAICS - that GDI deep copies what it needs out of the struct. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15276#issuecomment-1699664817
