RFR: 8308780: Fix the Java Integer types on Windows [v4]
Phil Race
prr at openjdk.org
Thu Jun 15 23:26:01 UTC 2023
On Thu, 1 Jun 2023 11:49:24 GMT, Julian Waters <jwaters at openjdk.org> wrote:
>> On Windows, the basic Java Integer types are defined as long and __int64 respectively. In particular, the former is rather problematic since it breaks compilation as the Visual C++ becomes stricter and more compliant with every release, which means the way Windows code treats long as a typedef for int is no longer correct, especially with -permissive- enabled. Instead of changing every piece of broken code to match the jint = long typedef, which is far too time consuming, we can instead change jint to an int (which is still the same 32 bit number type as long), as there are far fewer problems caused by this definition. It's better to get this over and done with sooner than later when a future version of Visual C++ finally starts to break on existing code
>
> Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix the code that is actually warning
src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp line 1089:
> 1087: entry_point();
> 1088: colorBits = (jint*)safe_Malloc(MAX_ICON_SIZE * MAX_ICON_SIZE * sizeof(jint));
> 1089: GetDIBits(dc, iconInfo.hbmColor, 0, iconSize, colorBits, &bmi, DIB_RGB_COLORS);
I just can't tell if the updates you are making as a result of the jni_md.h change are really the right ones and some of them don't look that way
You wrote
"As listed above, the native Windows API routines that the java.desktop code calls are actually expecting ints,"
Per the windows docs for GetDIBits() it takes an LPVOID for the parameter these are used for.
For the cases above if takes an LPVOID which is really just a void*, and using jint in the malloc is just weird since jint doesn't mean anything to GDI
And clearly this change requires running a whole load of client tests on windows [*]
but I have no idea what your reply to my question about that means. What is "-permissive" ?
[*] and I have no idea what VM etc tests need to be run just for the JNI change
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14125#discussion_r1231621441
More information about the core-libs-dev
mailing list