RFR: 8234741: enhance os::get_core_path on macOS

[Langer, Christoph]

Hi Matthias,

this looks good to me.

Please also add os:: to the other call to current_process_id() in line 3787 (the else branch). No need for another webrev, though.

/Christoph

> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> Baesken, Matthias
> Sent: Mittwoch, 27. November 2019 11:09
> To: gerard ziemski <gerard.ziemski at oracle.com>
> Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net>
> Subject: RE: RFR: 8234741: enhance os::get_core_path on macOS
> 
> Hi Gerard, thanks for your input .
> 
> New webrev :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8234741.1/
> 
> 
> Best regards, Matthias
> 
> 
> >
> > hi Matthias,
> >
> > I had to look up "kern.corefile" option, of which I was not previously
> > aware - it looks like a nice enhancement.
> >
> > I'd like to suggest a few small cleanups though:
> >
> > #1 add "os::" prefix to "current_process_id()" call
> > #2 restrict the scope of "os::current_process_id()" to only the branch
> > of "if" that needs it
> > #3 expand on the "tail" comment a bit to explain why it might be needed
> >
> > Perhaps something like this:
> >
> >    char coreinfo[MAX_PATH];
> >     size_t sz = sizeof(coreinfo);
> >     int ret = sysctlbyname("kern.corefile", coreinfo, &sz, NULL, 0);
> >     if (ret == 0) {
> >       char *pid_pos = strstr(coreinfo, "%P");
> >       const char* tail = (pid_pos != NULL) ? (pid_pos + 2) : ""; // skip
> > over the "%P" to preserve any optional custom user pattern (i.e. %N, %U)
> >       if (pid_pos != NULL) {
> >         *pid_pos = '\0';
> >         n = jio_snprintf(buffer, bufferSize, "%s%d%s", coreinfo,
> > os::current_process_id(), tail);
> >       } else {
> >         n = jio_snprintf(buffer, bufferSize, "%s", coreinfo);
> >       }
> >     }
> >
> > BTW. I'm glad you agree to remove the unrelated AWT change from this fix
> > and let the client team handle it.
> >
> >