RFR: JDK-8059586: hs_err report should treat redirected core pattern.
Yasumasa Suenaga
yasuenag at gmail.com
Tue Dec 9 13:56:42 UTC 2014
David, Thomas,
Thank you so much!
I wait 2nd reviewer.
BTW, I'm not a committer.
So I'm also waiting a sponsor :-)
> I'm okay with these changes. Just a minor style nit (no need for updated webrev) can you remove the blank lines in os_linux.cpp:
>
> 6011 }
> 6012
> 6013 }
> 6014
> 6015 }
>
> 6057 }
> 6058
> 6059 }
> 6060
> 6061 }
>
> If anyone has any objections please raise them asap.
I will upload new webrev which is fix them after reviewing.
Thanks,
Yasumasa
(2014/12/09 21:06), Thomas Stüfe wrote:
> Yes, Sure :-) @Yasumasa : thank you for this patch!
>
> Kind regards, Thomas
>
> On Dec 9, 2014 9:56 AM, "David Holmes" <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>
> Hi Thomas,
>
> So can we take this as-is for now and file a RFE to address your concerns?
>
> Anybody else object to that?
>
> Thanks,
> David
>
> On 9/12/2014 6:09 PM, Thomas Stüfe wrote:
>
> Hi David,
>
> On Tue, Dec 9, 2014 at 6:39 AM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
> <mailto:david.holmes at oracle.__com <mailto:david.holmes at oracle.com>>> wrote:
>
> Hi Thomas,
>
> On 8/12/2014 8:27 PM, Thomas Stüfe wrote:
>
> Hi,
>
> I do not really like the handling of the leading pipe symbol:
>
>
> To be fair to Yasumasa this aspect of the fix has been the same
> since Oct 15:
>
> http://cr.openjdk.java.net/~____ysuenaga/JDK-8059586/webrev.____02/ <http://cr.openjdk.java.net/~__ysuenaga/JDK-8059586/webrev.__02/>
> <http://cr.openjdk.java.net/~__ysuenaga/JDK-8059586/webrev.__02/ <http://cr.openjdk.java.net/~ysuenaga/JDK-8059586/webrev.02/>>
>
> and was not flagged.
>
>
> You are right, I did not read those mails close enough.
>
>
>
> So, we read the core_pattern, and if the pipe symbol is detected, we
> write the core pattern minus the pipe symbol but plus a leading
> quote to
> the output; the leading quote then serves as a info to the layer
> above
> in os_posix.cpp to treat this case specially. This means the logic
> spills out of the platform dependend os_linux.cpp to shared code and
> this is also difficult to read.
>
> This comes from the fact that "get_core_path()" assumes the core
> file is
> written to the file system. I think it just does not fit
> anymore, better
> would be to replace it with something like
> "os::print_core_file_location(____outputStream* os)", and the OS
> handles
> both core path retrieval and the printing. Because then the
> shared code
> does not need to know whether core file gets printed
> traditionally or
> piped to a executable or whatever.
>
>
> This sounds like a refactoring that I suggested would be too disruptive.
>
> http://mail.openjdk.java.net/____pipermail/hotspot-dev/2014-____October/015547.html <http://mail.openjdk.java.net/__pipermail/hotspot-dev/2014-__October/015547.html>
> <http://mail.openjdk.java.net/__pipermail/hotspot-dev/2014-__October/015547.html <http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-October/015547.html>>
>
> http://mail.openjdk.java.net/____pipermail/hotspot-dev/2014-____October/015557.html <http://mail.openjdk.java.net/__pipermail/hotspot-dev/2014-__October/015557.html>
> <http://mail.openjdk.java.net/__pipermail/hotspot-dev/2014-__October/015557.html <http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-October/015557.html>>
>
> http://mail.openjdk.java.net/____pipermail/hotspot-dev/2014-____October/015573.html <http://mail.openjdk.java.net/__pipermail/hotspot-dev/2014-__October/015573.html>
> <http://mail.openjdk.java.net/__pipermail/hotspot-dev/2014-__October/015573.html <http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-October/015573.html>>
>
>
> I do not think that this would be such a big a change, but it also could
> be done with another patch.
>
> Apart from my reservations I stated above the code looks fine and is
> definitly an improvement (just last week I was helplessly looking for a
> core on a machine where core_pattern turned out to be a redirection to
> another program).
>
> Kind Regards, Thomas
>
>
> David
>
More information about the hotspot-dev
mailing list