RFR: 8136978 Much nearly duplicated code for vmError support

Thomas Stüfe thomas.stuefe at gmail.com
Wed Nov 18 14:24:42 UTC 2015


Hi Sebastian,

looks fine, I will test on AIX. I assume Coleen will test the other
platforms.

..Thomas

On Wed, Nov 18, 2015 at 3:04 PM, Sebastian Sickelmann <
sebastian.sickelmann at gmx.de> wrote:

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