Hi David,

On 21/10/2019 02:19, David Holmes wrote:
Hi Alexey,

On 21/10/2019 9:37 am, Alexey Ivanov wrote:
Hi David,

On 20/10/2019 23:59, David Holmes wrote:
Hi Alexey,

On 21/10/2019 2:20 am, Alexey Ivanov wrote:
Hello,

Please review the following fix which it brings back NewStringPlatform alias for JNU_NewStringPlatform. Without it, 32 bit Windows build of Java does not work.

bug: https://bugs.openjdk.java.net/browse/JDK-8232624
webrev: http://cr.openjdk.java.net/~aivanov/8232624/webrev.00/

Not sure this is the correct fix. The problem as I see it is that NewStringPlatform was not declared as a JNICALL previously whereas the JNU_NewStringPLatform is. That affects the linkage on 32-bit Windows only.

Yes, JNICALL affects 32-bit Windows only. And this is exactly why "JNU_NewStringPLatform" cannot be found by its name on 32-bit Windows; "NewStringPlatform" is not declared as JNICALL and it can be found by its undecorated name on 32-bit Windows and all the other platforms. I believe it's the reason why this alias exists.

So what would happen if we drop the JNICALL from JNU_NewStringPLatform?

Yes, it will be found by its undecorated name too.
But I'd rather not change the calling conventions of functions with JNU_ prefix.

So, I think the proposed patch is the easiest and safest way to fix 32-bit Windows build.

Another way to fix it is to lookup the undecorated name first and, if it fails, to lookup the decorated name, which makes the code harder to read.

With this patch, I'm reverting the code to the state it was before.

Yes, but Claes didn't like the way it was before :) so I'm hoping we can keep his cleanup whilst still allowing Windows to work correctly.

Probably, Claes thought "NewStringPlatform" wasn't used. Yet it proved that "NewStringPlatform" is still used.

If required, I can create a follow-up issue to re-do the cleanup as Alan suggested.

--
Regards,
Alexey

Reply via email to