RFR(s): 8143291: Remove redundant coding around os::exception_name

David Holmes david.holmes at oracle.com
Tue Dec 1 23:10:44 UTC 2015


Hi Thomas,

On 30/11/2015 6:11 PM, Thomas Stüfe wrote:
> Hi Coleen, David,
>
> new webrev here:
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8143291/webrev.04/webrev/

You somehow reverted the changes in os_windows.cpp to those from version 
2 which failed to build. So I've taken version 3 and then applied just 
the jvm_solaris.cpp change from version 4. I checked the diff between v3 
and v4 to ensure no other changes.

Also note in the changset comments when you commit it needs to be 
"Reviewed-by" with lowercase 'b', else jcheck rejects it.

Resubmitting the push now.

Thanks,
David

> The only change are the deleted bogus comments in jvm_solaris.cpp Coleen
> did ask for.
>
> Kind Regards, Thaoms
>
> On Mon, Nov 30, 2015 at 5:27 AM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
>     On 29/11/2015 3:33 AM, Coleen Phillimore wrote:
>
>
>         I am on US Thanksgiving holiday but this looks great.  I'm happy
>         to see
>         these cleanups.  It'll make working on the code better.
>
>         Can you remove the bogus
>
>            //Reconciliation History
>
>
>         lines from jvm_solaris.cpp while you're here?
>
>
>     I can do that before pushing unless Thomas gets to it first. Due to
>     internal constraints I have to wait until after Monday to push
>     anything at the moment.
>
>         Are JVM_RaiseSignal and JVM_RegisterSignal on xnix platforms mostly
>         duplicate now?  A further RFE?
>
>
>     Yes a further RFE. A lot of the signal related code could be
>     simplified and refactored I think.
>
>     Thanks,
>     David
>
>
>         Thanks,
>         Coleen
>
>         On 11/27/15 11:04 AM, Thomas Stüfe wrote:
>
>             Ping...
>
>             Could I have a second review please?
>
>             The current webrev is:
>             http://cr.openjdk.java.net/~stuefe/webrevs/8143291/webrev.03/webrev/
>             And the bug report:
>             https://bugs.openjdk.java.net/browse/JDK-8143291
>
>             Thank you!
>
>             ..Thomas
>
>
>             On Thu, Nov 19, 2015 at 11:23 AM, Thomas Stüfe
>             <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>>
>             wrote:
>
>                 Hi,
>
>                 please take a look at this change.
>
>                 bug: https://bugs.openjdk.java.net/browse/JDK-8143291
>                 webrev:
>                 http://cr.openjdk.java.net/~stuefe/webrevs/8143291/webrev.00/webrev/
>
>                 This fix does some cleanups around os::exception_name().
>
>                 - os::exception_name() is identical on all Posix
>                 platforms (it returns a
>                 signal name string for a signal number), so it can be
>                 merged into
>                 os_posix.cpp
>
>                 - There is no need for a platform-specific
>                 implementation, as we have
>                 already os::Posix::get_signal_name(). Use that instead of
>                 platform-specific
>                 solutions.
>
>                 - I added a function os::Posix::get_signal_number()
>                 which is the inverse
>                 of os::Posix::get_signal_name().
>
>                 - Before, a signal-to-number table was kept in
>                 jvm_<os>.cpp. That was
>                 used
>                 to implement os::exception_name() and also for
>                 JVM_FindSignal
>                 -> on AIX, I removed the coding altogether and used
>                 os::Posix::get_signal_number() as a base for JVM_FindSignal.
>                 -> on the other Unices, I did not feel so confident,
>                 because strictly
>                 spoken we may change the behaviour slightly to before:
>                 os::Posix::get_signal_name() knows more signal names
>                 than the platform
>                 specific tables knew before, so now
>                 Signal.findSignal("<name>") would
>                 return more matches than before.
>                 I am not sure whether I am overcautious here - should I
>                 treat the other
>                 Unices the same way I treated AIX, i.e. implement
>                 Signal.findSignal("<name>") -> JVM_FindSignal via
>                 os::Posix::get_signal_number()? This would further
>                 simplify the coding.
>
>                 Oh, this fix also fixes an issue where
>                 os::exception_name() would return
>                 NULL for an unknown signal number, but no caller ever
>                 checks for NULL
>                 before printing the result. The new os::exception_name()
>                 always
>                 returns a
>                 string and also distinguishes between "unknown but valid
>                 signal" and
>                 "invalid signal".
>
>                 Kind Regards, Thomas
>
>
>
>
>


More information about the hotspot-runtime-dev mailing list