On Thu, 1 Jun 2023 11:49:24 GMT, Julian Waters <jwat...@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

Reply via email to