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]

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