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

[Thomas Stüfe]

Hi David,


On Thu, Nov 19, 2015 at 12:56 PM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
> I like the idea of this, but it will take me a little time to check all
> the details. More below ...
>
> On 19/11/2015 8:23 PM, Thomas Stüfe 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.
>>
>
> How so? If the additional names are platform specific then they won't
> exist at build time on the other platforms and so will be elided from the
> table.
>
> BTW the existing Solaris code is incorrect when we start building on
> Solaris 11, so it would be good to use something that uses the actual build
> time signal value rather than the current array based approach.
>
>
Okay, I see. Then it really makes sense to discard the signal name tables
in the various jvm_<os>.cpp files in favor of os::Posix::get_signal_number
like I did on AIX. Will do tomorrow and post updated webrev.

Thanks for reviewing!

..Thomas



> Thanks,
> David
>
>
> 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
>>
>>