On 22/02/2019 4:55 am, Mikael Vidstedt wrote:
The wrapper based solution looks much cleaner to me as well. webrev.01 looks
good.
Sorry I really don't like it. The wrappers obfuscate and make
complicated something that is not at all complicated. I have to re-read
the new code 3 times to figure out what it is even doing!
All that complexity to handle the fact one platform wants to return int
instead of void* ??
David
-----
(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
