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

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


Thank you Colleen!

On Sat, Nov 28, 2015 at 6:33 PM, Coleen Phillimore <
coleen.phillimore at oracle.com> 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?
>
>
Will do once I'm in office, unless David beats me to it.


> Are JVM_RaiseSignal and JVM_RegisterSignal on xnix platforms mostly
> duplicate now?  A further RFE?
>
>
Yes, I think so. A lot of cleanup is possible. I'll try to get some more
cleanups throigh before feature close.

Kind Regards, Thomas


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


More information about the hotspot-runtime-dev mailing list