8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Alexey Ivanov
alexey.ivanov at oracle.com
Thu Apr 12 21:40:20 UTC 2018
On 12/04/2018 19:38, Phil Race wrote:
> I was just directed to this look at this change.
> I don't know why it is being reviewed exclusively on build-dev since no
> build files are being changed.
My bad! I tried to engage core-libs when the patch was ready but I
completely overlooked the fact that 2d libs were also affected by this
change and I didn't cc to 2d-dev.
> 50% of it should have been sent to 2d-dev and the rest on core-libs +
> hotspot ..
>
> Is this the current version of the change :
> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ ?
Yes, it's the latest version to the best of my knowledge.
Regards,
Alexey
>
> -phil.
>
> On 04/12/2018 12:49 AM, Baesken, Matthias wrote:
>> Hi, could someone please sponsor the change now ?
>>
>> And could someone please check what happened to the submit-repo ?
>> Yesterday I pushed to the submit repo to check my change , but
>> no response so far .
>> Maybe the submit repo is not working currently , not sure about it .
>>
>>
>> Best regards , Matthias
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Baesken, Matthias
>>> Sent: Mittwoch, 11. April 2018 11:20
>>> To: 'Alexey Ivanov' <alexey.ivanov at oracle.com>; Magnus Ihse Bursie
>>> <magnus.ihse.bursie at oracle.com>
>>> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
>>> <martin.doerr at sap.com>
>>> 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
>>>
>>>> Was main() exported via map files?
>>>>
>>> Seems main was exported , I can find it in jdk10 in e.g. :
>>>
>>> make/mapfiles/launchers/mapfile-sparcv9
>>> make/mapfiles/launchers/mapfile-x86_64
>>>
>>>
>>> Best regards, Matthias
>>>
>>>
>>>> -----Original Message-----
>>>> From: Alexey Ivanov [mailto:alexey.ivanov at oracle.com]
>>>> Sent: Mittwoch, 11. April 2018 11:11
>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Magnus Ihse Bursie
>>>> <magnus.ihse.bursie at oracle.com>
>>>> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
>>>> <martin.doerr at sap.com>
>>>> 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
>>>>
>>>>
>>>> On 11/04/2018 08:44, Baesken, Matthias wrote:
>>>>>> JIMAGE_FindResource doesn't have JNICALL modifier now, does it?
>>>>> Hi Alexey, yes that's true .
>>>>>
>>>>>> Please remove JNIEXPORT from main():
>>>>>> src/java.base/share/native/launcher/main.c
>>>>>> src/jdk.pack/share/native/unpack200/main.cpp
>>>>> I would prefer to keep it for now .
>>>>> I notice some comments in our SAPJVM code base about needing
>>>> JNIEXPORT for main for Solaris (we were running in SAPJVM without
>>>> mapfiles in the past already).
>>>>> Maybe that’s related to
>>>>>
>>>>> src/java.base/unix/native/libjli/java_md_solinux.c
>>>>>
>>>>> where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT,
>>>>> "main");
>>>>> but I am not sure about this.
>>>>> So I better keep the JNIEXPORT for the main functions, could be
>>>> removed in another cleanup if really needed.
>>>>
>>>> OK. Let them stay then.
>>>> Was main() exported via map files?
>>>>
>>>>
>>>> The change looks good to me.
>>>>
>>>> Regards,
>>>> Alexey
>>>>
>>>>>> You can reference both yourself and me as
>>>>>> Contributed-by: mbaesken, aivanov
>>>>>> when pushing the changeset if you don't mind.
>>>>>>
>>>>> Sure .
>>>>>
>>>>> Best regards, Matthias
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Alexey Ivanov [mailto:alexey.ivanov at oracle.com]
>>>>>> Sent: Dienstag, 10. April 2018 21:34
>>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Magnus Ihse
>>>> Bursie
>>>>>> <magnus.ihse.bursie at oracle.com>
>>>>>> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
>>>>>> <martin.doerr at sap.com>
>>>>>> 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
>>>>>>
>>>>>> Hi Matthias,
>>>>>>
>>>>>> On 10/04/2018 11:14, Baesken, Matthias wrote:
>>>>>>> Hello, I had to do another small adjustment to make
>>>>>>> jimage.hpp/cpp
>>>> match. Please review :
>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/
>>>>>> JIMAGE_FindResource doesn't have JNICALL modifier now, does it?
>>>>>>
>>>>>> I've successfully built 32 bit Windows with your patch.
>>>>>>
>>>>>>
>>>>>> Please remove JNIEXPORT from main():
>>>>>> src/java.base/share/native/launcher/main.c
>>>>>> src/jdk.pack/share/native/unpack200/main.cpp
>>>>>>
>>>>>>> With the latest webrev I could finally build jdk/jdk
>>>>>>> successfully on both
>>>> win32bit and win64 bit.
>>>>>>> Thanks again to Alexey to provide the incorporated patch .
>>>>>> You can reference both yourself and me as
>>>>>> Contributed-by: mbaesken, aivanov
>>>>>> when pushing the changeset if you don't mind.
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Alexey
>>>>>>
>>>>>>> Best regards, Matthias
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Alexey Ivanov [mailto:alexey.ivanov at oracle.com]
>>>>>>>> Sent: Montag, 9. April 2018 17:14
>>>>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Magnus Ihse
>>>>>> Bursie
>>>>>>>> <magnus.ihse.bursie at oracle.com>
>>>>>>>> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
>>>>>>>> <martin.doerr at sap.com>
>>>>>>>> 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
>>>>>>>>
>>>>>>>> Hi Matthias,
>>>>>>>>
>>>>>>>> On 09/04/2018 15:38, Baesken, Matthias wrote:
>>>>>>>>> Hi Alexey, thanks for the diff provided by you, and for
>>>>>>>>> the
>>>>>> explanations
>>>>>>>> .
>>>>>>>>> I created a second webrev :
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/
>>>>>>>>>
>>>>>>>>> - it adds the diff provided by you (hope that’s fine
>>>>>>>>> with you)
>>>>>>>> Yes, that's fine with me.
>>>>>>>> There could be only one author ;)
>>>>>>>>
>>>>>>>>> - changes 2 launchers
>>> src/java.base/share/native/launcher/main.c
>>>>>> and
>>>>>>>> src/jdk.pack/share/native/unpack200/main.cpp where we face
>>>> similar
>>>>>>>> issues after mapfile removal for exes
>>>>>>>>
>>>>>>>> I'd rather remove both JNIEXPORT and JNICALL from main().
>>>>>>>> It wasn't exported, and it shouldn't be.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Alexey
>>>>>>>>
>>>>>>>>> Best regards , Matthias
>
More information about the build-dev
mailing list