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

David Holmes david.holmes at oracle.com
Wed Dec 16 06:09:02 UTC 2015


Changes pushed.

Thanks Thomas!

Just FYI I finish for the Xmas break at the end of this week, back on 
January 6. A lot of folk are on break over Xmas/NewYear so things will 
be quiet here. I plan to look in from time to time :)

David

On 15/12/2015 8:35 PM, Thomas Stüfe wrote:
> 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 <mailto: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 <mailto: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 <mailto: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
>     <mailto: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