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

Thomas Stüfe thomas.stuefe at gmail.com
Mon Nov 30 06:58:07 UTC 2015

Hi David,

On Mon, Nov 30, 2015 at 5:27 AM, David Holmes <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 jvmsolaris.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.

If you get to it, go ahead, otherwise I'll do a new wevrev once I'm in office.

Are JVMRaiseSignal and JVMRegisterSignal 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. I'm currently preparing a cleanup for Posix os::print_siginfo, and I think further cleanups are possible.

.. Thoams

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> 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::exceptionname(). - os::exceptionname() is identical on all Posix platforms (it returns a signal name string for a signal number), so it can be merged into osposix.cpp - There is no need for a platform-specific implementation, as we have already os::Posix::getsignalname(). Use that instead of platform-specific solutions. - I added a function os::Posix::getsignalnumber() which is the inverse of os::Posix::getsignalname(). - Before, a signal-to-number table was kept in jvm.cpp. That was used to implement os::exceptionname() and also for JVMFindSignal -> on AIX, I removed the coding altogether and used os::Posix::getsignalnumber() as a base for JVMFindSignal. -> on the other Unices, I did not feel so confident, because strictly spoken we may change the behaviour slightly to before: os::Posix::getsignalname() knows more signal names than the platform specific tables knew before, so now Signal.findSignal("") 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("") -> JVMFindSignal via os::Posix::getsignalnumber()? This would further simplify the coding. Oh, this fix also fixes an issue where os::exceptionname() would return NULL for an unknown signal number, but no caller ever checks for NULL before printing the result. The new os::exceptionname() 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