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

[Thomas Stüfe]

Hi David,

On Wed, Dec 2, 2015 at 12:10 AM, David Holmes <david.holmes at oracle.com>
wrote:

> 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.
>
>
Thanks a lot and sorry for the trouble! I guess I am juggling too many
changes. Will be more careful next time.


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

Thanks for correcting.

I am working with mercurial queues and jcheck does not fully work with
those. I will see how I can improve this.

Kind Regards, Thomas


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