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 >>>>>>>
