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

[Thomas Stüfe]

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


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


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