RFR: 8136978 Much nearly duplicated code for vmError support

Sebastian Sickelmann sebastian.sickelmann at gmx.de
Wed Nov 18 14:04:56 UTC 2015


Hi Thomas,



On 11/18/2015 10:52 AM, Thomas Stüfe wrote:
> Hi Sebastian,
>
> Your patch fails to apply on my repository. I tested against a freshly
> pulled jdk9/dev. I think it may conflict with "8080775: Better
> argument formatting for assert() and friends".
>
> --------------
>
> From looking at it, this change is a nice cleanup!
>
> Here are some few cosmetic issues:
>
> - os::formatDebugMessage() and void os::startDebugging():
>
> Standard naming scheme would be lowercase with underscores. Could you
> rename them "format_debug_message" and "start_debugging", respectivly?
DONE
>
> -
> http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/src/os/posix/vm/os_posix.hpp.udiff.html
> <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/src/os/posix/vm/os_posix.hpp.udiff.html>
>
> Could you please group
>
>   static int unblock_signals(const sigset_t *set);
>
> with the other signal set functions above and maybe also add a comment?
DONE
>
> ---
>
> Also, I would like to see in the name of "unblock_signals" reflected
> that this is thread-specific, e.g. "unblock_thread_signals".
DONE
>
> Or even better, a more generic function which both blocks and unblocks
> signals for the calling thread, e.g.
> "os::Posix::set_thread_signal_mask(const sigset_t *set, bool do_block)".
I like this idea. But i think it is another general cleanup that goes to
many sources. Meanwhile I will create a ticket for it.

Please find the other mail where i posted the webrev-url and describe
some things regarding the "Reviewed-by" line.

-- 
Sebastian
>
> ---
>
> Kind Regards, Thomas
>
>



More information about the hotspot-runtime-dev mailing list