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

[David Holmes]

Hi Yasumasa,

On 7/10/2014 8:48 PM, Yasumasa Suenaga wrote:
> Hi David,
>
> Sorry for my English.
>
> I want to propose that JVM should create message according to core
> pattern (/proc/sys/kernel/core_pattern) .
> So I filed it to JBS and created a patch.

So I've had a quick look at this core_pattern business and it seems to 
me that there are two aspects to this.

First, without the leading |, the entry in the core_pattern file is a 
naming pattern for the core file. In which case that should be handled 
by the linux specific get_core_path() function. Though that in itself 
can't fully report the expected name, as part of it is provided in the 
shared code in os::check_or_create_dump. Fixing this means changing all 
the os_posix using platforms. But your patch is not about this part. :)

Second, with a leading | the core_pattern is actually the name of a 
program to execute when the program is about to core dump, and that is 
what you report with your patch. Though I'm unclear whether it both 
invokes the program and creates a core dump file; or just invokes the 
program?

So with regards to this second part your patch seems functionally ok. I 
do dislike having a big chunk of linux specific code in this "posix" 
support file but ...

A few style nits - you need spaces around keywords and before braces eg:

   if(x){

should be

   if (x) {

I also suggest saying "Core dumps may be processed with ..." rather than 
"treated".

And as you don't do anything in the non-redirect case I suggest 
collapsing this:

   83           is_redirect = core_pattern[0] == '|';
   84         }
   85
   86         if(is_redirect){
   87           jio_snprintf(buffer, bufferSize,
   88                    "Core dumps may be treated with \"%s\"", 
&core_pattern[1]);
   89         }

to just

   83           if (core_pattern[0] == '|') {  // redirect
   84             jio_snprintf(buffer, bufferSize, "Core dumps may be 
processed with \"%s\"", &core_pattern[1]);
   85            }
   86         }

Comments from other runtime folk appreciated.

Thanks,
David

> Thanks,
>
> Yasumasa
>
> 2014/10/07 15:43 "David Holmes" <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>>:
>
>     Hi Yasumasa,
>
>     I'm sorry but I don't understand what you are proposing. When you say
>     "treat" do you mean "create"? Otherwise what do you mean by "treated"?
>
>     Thanks,
>     David
>
>     On 2/10/2014 8:38 AM, Yasumasa Suenaga wrote:
>      > I'm in Hackergarten @ JavaOne :-)
>      >
>      >
>      > Hi all,
>      >
>      > I would like to enhance the messages in hs_err report.
>      > Modern Linux kernel can treat core dump with user process (e.g. ABRT)
>      > However, hs_err report cannot detect it.
>      >
>      > I think that hs_err report should output messages as below:
>      > -------------
>      >     Failed to write core dump. Core dumps may be treated with
>     "/usr/sbin/chroot /proc/%P/root /usr/libexec/abrt-hook-ccpp %s %c %p
>     %u %g %t e"
>      > -------------
>      >
>      > I've uploaded webrev of this enhancement.
>      > Could you review it?
>      >
>      > http://cr.openjdk.java.net/~ysuenaga/JDK-8059586/webrev.00/
>      >
>      > This patch works fine on Fedora20 x86_64.
>      >
>      >
>      >
>      > Thanks,
>      >
>      > Yasumasa
>      >
>