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]

I guess we also need to get an approval from core-libs (cc'd) as libzip 
and libjimage are modified.


Regards,
Alexey

On 11/04/2018 10:56, Alexey Ivanov wrote:
> The change looks good to me.
>
> On 11/04/2018 10:20, Baesken, Matthias wrote:
>>> 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
>
> Thank you for confirming it.
> Then JNIEXPORT is better left untouched to keep main() exported.
>
>
> Regards,
> Alexey
>
>> 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
>