RFR(s): 8144219: [posix] Remove redundant code around os::print_siginfo()
Thomas Stüfe
thomas.stuefe at gmail.com
Tue Dec 15 10:35:37 UTC 2015
Hi Volker,
thanks for reviewing!
Latest webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8144219-posix-os-print_siginfo/webrev.05/webrev/index.html
On Mon, Dec 14, 2015 at 5:39 PM, Volker Simonis <volker.simonis at gmail.com>
wrote:
> 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?
>
>
Just an oversight - I removed the prototype.
> Regards,
> Volker
>
>
>
@David: now that we have two reviews, this could be pushed? Thanks!
>
> 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