On Thu, 15 Jun 2023 23:22:48 GMT, Phil Race <p...@openjdk.org> wrote:
>> 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 -permissive- is a compiler switch that forces the Microsoft Visual C compiler to be stricter in compiling C and C++ and makes it enforce the standard much more aggressively. It's becoming less permissive with every iteration of the Microsoft compiler and is stated to become enabled by default eventually by Microsoft. One of the consequences of this is that in the future our code cannot so loosely treat int and long as the same type on Windows (even though they are ultimately the same size in compiled code), as far as the compiler is concerned, they are semantically 2 entirely different types. That's the complication this Pull Request is trying to preempt by changing the jni.h typedef for Windows I missed the LPVOID change in GetDIBits, but for the other changes I really only followed the existing declarations, the jint for colorBits is because the call to SetIntArrayRegion takes a jint as a parameter. Let me know if I should change the declaration regardless, though Also, we have David for the VM reviews ;) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14125#discussion_r1231658403
