Hi Alan , this is the up to date webrev . However we want to add Alexey Ivanov as additional author .
> > As I read it, this changes the calling convention of these functions on > 32-bit Windows but it will have no impact on 64-bit Windows (as > __stdcall is ignored) or other platforms, is that correct? > The change adds JNIEXPORT at some places where it is ben forgotten , for example : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/src/java.desktop/share/native/libmlib_image/mlib_c_ImageLookUp.c.udiff.html This might have potential impact on other platforms (fixes the mismatches) . Best regards, Matthias > -----Original Message----- > From: Alan Bateman [mailto:alan.bate...@oracle.com] > Sent: Donnerstag, 12. April 2018 12:54 > To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> > Cc: build-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>; > core-libs-...@openjdk.java.net > Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in function > declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at > some places in function declarations/implementations > > Adding core-libs-dev as this is change code in libjava, libzip, > libjimage, ... > > Can you confirm that this is the up to date webrev: > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ > > As I read it, this changes the calling convention of these functions on > 32-bit Windows but it will have no impact on 64-bit Windows (as > __stdcall is ignored) or other platforms, is that correct? > > -Alan > > > On 06/04/2018 14:20, Baesken, Matthias wrote: > > Hello, I just noticed 2 additonal issues regarding mapfile-removal : > > > > > > 1. The follow up change that removed mapfiles for exes as well > leads on Win32 bit to this link error : > > > > Creating library > C:/JVM/jdk_jdk_ntintel/support/native/java.base/java/java.lib and object > C:/JVM/jdk_jdk_ntintel/support/native/java.base/java/java.exp > > LIBCMT.lib(crt0.obj) : error LNK2019: unresolved external symbol _main > referenced in function ___tmainCRTStartup > > C:/JVM/jdk_jdk_ntintel/support/native/java.base/java_objs/java.exe : > fatal error LNK1120: 1 unresolved externals > > > > > > Looks like we cannot have JNICALL in main.c : > > > > diff -r 4f6887eade94 src/java.base/share/native/launcher/main.c > > --- a/src/java.base/share/native/launcher/main.c Thu Apr 05 14:39:04 > 2018 -0700 > > +++ b/src/java.base/share/native/launcher/main.c Fri Apr 06 15:16:40 > 2018 +0200 > > @@ -93,7 +93,7 @@ > > __initenv = _environ; > > > > #else /* JAVAW */ > > -JNIEXPORT int JNICALL > > +JNIEXPORT int > > main(int argc, char **argv) > > { > > int margc; > > > > > > > > 1. Zip-library has runtime issues when symbols are resolved in > src/hotspot/share/classfile/classLoader.cpp. > > > > I added the declaration of the missing symbol, this seems to fix it . > > > > > > diff -r 4f6887eade94 src/java.base/share/native/libzip/zip_util.h > > --- a/src/java.base/share/native/libzip/zip_util.h Thu Apr 05 14:39:04 > 2018 -0700 > > +++ b/src/java.base/share/native/libzip/zip_util.h Fri Apr 06 15:16:40 > 2018 +0200 > > @@ -284,4 +284,7 @@ > > JNIEXPORT jboolean JNICALL > > ZIP_InflateFully(void *inBuf, jlong inLen, void *outBuf, jlong outLen, char > **pmsg); > > > > +JNIEXPORT jint JNICALL > > +ZIP_CRC32(jint crc, const jbyte *buf, jint len); > > + > > > > > > Should I prepare another bug, or add this to the existing one and > > post a > second webrev ? > > > > Best regards, Matthias > > > > > > From: Baesken, Matthias > > Sent: Freitag, 6. April 2018 09:54 > > To: 'Magnus Ihse Bursie' <magnus.ihse.bur...@oracle.com> > > Cc: build-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com> > > Subject: RFR: 8201226 missing JNIEXPORT / JNICALL at some places in > function declarations/implementations - was : RE: missing JNIEXPORT / > JNICALL at some places in function declarations/implementations > > > > Hello, please review : > > > > Bug : > > > > https://bugs.openjdk.java.net/browse/JDK-8201226 > > > > change : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ > > > > mostly I added JNIEXPORT / JNICALL at some places where the win32bit > build missed it . > > > > A difference is > src/java.desktop/share/native/libsplashscreen/splashscreen_impl.h > > Where I removed a few declarations (one decl with one without > JNIEXPORT / JNICALL – looked a bit strange ) . > > > > Best regards , Matthias > > > > > > From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] > > Sent: Donnerstag, 5. April 2018 17:45 > > To: Baesken, Matthias > <matthias.baes...@sap.com<mailto:matthias.baes...@sap.com>> > > Cc: build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>; > Doerr, Martin <martin.do...@sap.com<mailto:martin.do...@sap.com>> > > Subject: Re: missing JNIEXPORT / JNICALL at some places in function > declarations/implementations > > > > That's most likely a result of the new JNIEXPORT I added as part of the > mapfile removal. > > > > I tried to match header file and C file, but I can certainly have missed > > cases. > If I didn't get any warnings, it was hard to know what I missed. > > > > Please do submit your patch. > > > > I'm a bit surprised 32-bit Window is still buildable. :) > > > > /Magnus > > > > 5 apr. 2018 kl. 17:20 skrev Baesken, Matthias > <matthias.baes...@sap.com<mailto:matthias.baes...@sap.com>>: > > Hello, we noticed that at a number of places in the coding , the > JNIEXPORT and/or JNICALL modifiers do not match when one compares > the declaration and > > The implementation of functions. > > While this is ok on most platforms, it seems to fail on Windows 32 bit and > leads to errors like this one : > > > > > e:/priv/openjdk/repos/jdk/src/java.desktop/share/native/libmlib_image/ml > ib_ImageConvKernelConvert.c(87) : error C2373: > 'j2d_mlib_ImageConvKernelConvert' : redefinition; different type modifiers > > > e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\ml > ib_image_proto.h(2630) : see declaration of > 'j2d_mlib_ImageConvKernelConvert' > > > > (there are quite a few of these e.g. in mlib / splashscreen etc.) > > > > > > Another example is this one : > > > > diff -r 4d98473ed33e src/java.base/share/native/libjimage/jimage.hpp > > --- a/src/java.base/share/native/libjimage/jimage.hpp Thu Apr 05 09:55:16 > 2018 +0200 > > +++ b/src/java.base/share/native/libjimage/jimage.hpp Thu Apr > > 05 > 17:07:40 2018 +0200 > > @@ -126,7 +126,7 @@ > > * JImageLocationRef location = (*JImageFindResource)(image, > > * "java.base", "9.0", > > "java/lang/String.class", &size); > > */ > > -extern "C" JNIEXPORT JImageLocationRef > JIMAGE_FindResource(JImageFile* jimage, > > +extern "C" JNIEXPORT JImageLocationRef JNICALL > JIMAGE_FindResource(JImageFile* jimage, > > const char* module_name, const char* version, const char* name, > > jlong* size); > > > > > > Is there some generic way to get the same declarations / impementations > in the code ? > > > > Or should I just add a patch with my findings ? > > > > Best regards, Matthias > >