RFR(s): 8144219: [posix] Remove redundant code around os::print_siginfo()

Volker Simonis volker.simonis at gmail.com
Mon Dec 14 16:39:17 UTC 2015


Hi Thomas,

the change looks good. The only thing I don't understand is why we
still need os::Posix::print_siginfo():

  76   // A POSIX conform, platform-independend siginfo print routine.
  77   static void print_siginfo(outputStream* os, const siginfo_t* si);

I think it is never defined and not called either, or did I miss something?

Regards,
Volker



On Mon, Dec 14, 2015 at 3:50 PM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> Hi all,
>
> still need a second reviewer.
>
> The bug: https://bugs.openjdk.java.net/browse/JDK-8144219
> The webrev (rebased to current hs-rt):
> http://cr.openjdk.java.net/~stuefe/webrevs/8144219-posix-os-print_siginfo/webrev.04/webrev/
> <http://cr.openjdk.java.net/~stuefe/webrevs/8144219-posix-os-print_siginfo/webrev.00/webrev/>
>
> Thank you!
>
> ..Thomas
>
>
> On Sun, Dec 13, 2015 at 8:55 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>> May I have a second review, please?
>>
>> Thank you!
>> On Nov 30, 2015 11:03, "Thomas Stüfe" <thomas.stuefe at gmail.com> wrote:
>>
>>> Hi all,
>>>
>>> please review this small change:
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8144219-posix-os-print_siginfo/webrev.00/webrev/
>>> Bug Report: https://bugs.openjdk.java.net/browse/JDK-8144219
>>>
>>> This is mainly a cleanup change for posix platforms. It gets rid of
>>> multiple copies of os::print_siginfo() in favour of the central
>>> os::Posix::print_siginfo() in os_posix.cpp.
>>>
>>> It also extends os::Posix::print_siginfo() to write more details about
>>> the received signal which stems from our experiences with various Unices
>>> over the time. This will improve hs-err file printout.
>>>
>>> Note that there was on all platforms code which examines the crash
>>> address for a possible CDS fault and prints a specialized hint. In my
>>> opinion that makes no sense in a general os::print_siginfo() function and I
>>> moved it to a separate error reporting step into vmError.cpp. This may be a
>>> point of contention because the way to extract the fault address from a
>>> siginfo/EXCEPTION_RECORD structure is platform dependend, but imho this is
>>> still the least ugly solution.
>>>
>>> Kind regards, Thomas
>>>
>>>


More information about the hotspot-runtime-dev mailing list