RFR: JDK-8059586: hs_err report should treat redirected core pattern.

[David Holmes]

Sorry, I've been advised I need a second reviewer with an openjdk user 
name and as Staffan has not come back to this thread, that means I need 
one more reviewer from the runtime team please. And please check the 
email trail on this as it is quite long and I really don't want to 
revisit covered ground. :) Thanks.

David

On 10/12/2014 4:37 PM, David Holmes wrote:
> Hi Yasumasa,
>
> On 9/12/2014 11:56 PM, Yasumasa Suenaga wrote:
>> David, Thomas,
>>
>> Thank you so much!
>> I wait 2nd reviewer.
>
> I'm a Reviewer and I think Thomas counts as a reviewer. Plus Staffan had
> a look too. So I think this is good to go - though I'll give it till my
> morning before finalizing it.
>
>> BTW, I'm not a committer.
>> So I'm also waiting a sponsor :-)
>
> I will sponsor if you can prepare the changeset.
>
> Thanks,
> David
>
>>> 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
>>>