The wrapper based solution looks much cleaner to me as well. webrev.01 looks 
good.

(not a Reviewer)

Cheers,
Mikael

> On Feb 21, 2019, at 9:55 AM, Dmitry Chuyko <dmitry.chu...@bell-sw.com> wrote:
> 
> To me thread function wrappers look preferable to platform specific JavaMain 
> signature. Consider this webrev with wrappers:
> 
> http://cr.openjdk.java.net/~dchuyko/8215009/webrev.01/
> 
> In some cases JavaMain is called in the same thread and its result is 
> returned from JLI_Launch. ContinueInNewThread is in shared code. And JavaMain 
> uses macro controlled returns.
> So when JavaMain returns THREAD_FUNC_RETURN changes may contain some quite 
> artificial macro parts in java.c:
> 
> http://cr.openjdk.java.net/~dchuyko/8215009/webrev.02/
> 
> -Dmitry
> 
> On 12/19/18 9:27 AM, David Holmes wrote:
>> On 19/12/2018 1:56 am, Dmitry Chuyko wrote:
>>> On 12/18/18 3:39 AM, David Holmes wrote:
>>>> On 11/12/2018 9:30 pm, Dmitry Chuyko wrote:
>>>>> On 12/11/18 4:03 AM, David Holmes wrote:
>>>>>> Hi Dmitry,
>>>>>> 
>>>>>> On 11/12/2018 12:16 am, Dmitry Chuyko wrote:
>>>>>>> Hello,
>>>>>>> 
>>>>>>> Please review a small fix in java_md_solinux.c: continuation is not 
>>>>>>> truly compatible with pthread_create start_routine's signature but we 
>>>>>>> control what actually happens. So it makes sense to add intermediate 
>>>>>>> void* cast to silence the error.
>>>>>> 
>>>>>> I'd be tempted to fix the signature and get rid of all the casts.
>>>>> 
>>>>> David, the signature is a signature of
>>>>> 
>>>>> int JNICALL JavaMain(void * _args)
>>>>> 
>>>>> It would be fun to change it. But still on Windows it is correctly passed 
>>>>> to _beginthreadex() and then return code is extracted with 
>>>>> GetExitCodeThread(). In case we want it to return void* the cast will 
>>>>> move there.
>>>> 
>>>> I think the current double cast is truly ugly and an ifdef for windows, or 
>>>> a cast for Windows only would be an improvement.
>>> 
>>> I agree. Maybe making a wrapper function is not so ugly. If there are no 
>>> objections to changing beginning of the call stack it is quite easy to 
>>> implement. For consistency it may be done for all 3 points (posix unix, 
>>> posix mac, windows) or just for posix ones.
>>> 
>>> It looks like ifdef should be better as long as there are already 
>>> OS-specific parts in libjli. Again, if there are no objections to have 
>>> different JavaMain signatures on different platforms. In this case there 
>>> won't be a signature cast for Windows.
>> 
>> How about setting
>> 
>> #define THREAD_FUNC_RETURN int
>> 
>> in windows/java_md.h.
>> 
>> Then:
>> 
>> #ifndef THREAD_FUNC_RETURN
>>   #define THREAD_FUNC_RETURN void*
>> #endif
>> 
>> in java.h (after the other includes).
>> 
>> Then:
>> 
>> THREAD_FUNC_RETURN JNICALL
>> JavaMain(void * _args)
>> 
>> in java.c.
>> 
>> ?
>> 
>> Cheers,
>> David
>> 
>> 
>>> 
>>> -Dmitry
>>> 
>>>> 
>>>> But I won't impose that on you just to silence gcc 8.
>>>> 
>>>> Cheers,
>>>> David
>>>> 
>>>>> -Dmitry
>>>>> 
>>>>>> 
>>>>>> Cheers,
>>>>>> David
>>>>>> 
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8215009
>>>>>>> webrev: http://cr.openjdk.java.net/~dchuyko/8215009/webrev.00/
>>>>>>> testing: submit repo 
>>>>>>> (mach5-one-dchuyko-JDK-8215009-20181207-1625-13615: PASSED)
>>>>>>> 
>>>>>>> -Dmitry
>>>>>>> 

Reply via email to