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

[David Holmes]

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
>>