RFR(s): 8143291: Remove redundant coding around os::exception_name
David Holmes
david.holmes at oracle.com
Tue Dec 1 20:58:53 UTC 2015
Thanks Thomas. Changes being pushed now.
David
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/
>
> 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