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-runtime-dev mailing list