RFR: 8234741: enhance os::get_core_path on macOS

[gerard ziemski]

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.


cheers

On 11/26/19 2:36 AM, Baesken, Matthias wrote:
> OK I'll do a separate change .
>
> Are you fine with the rest of the patch ?
>
> Best regards, Matthias
>
>
>
>> On 25/11/2019 11:44 pm, Baesken, Matthias wrote:
>>> Hello,
>>>
>>> Currently the macOS implementation of os::get_core_path just displays
>> the default core file location.
>>> However it does not handle/show other locations set by the sysctl
>> parameter "kern.corefile" . This is enhanced by this change .
>>> I also take care of  handling %P  which is used a lot for  the  pid-placeholder
>> on macOS in the  "kern.corefile"  parameter .
>>>
>>> ( additionally  the change  contains a one-liner adjustment in
>> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h  to be able
>> to compile  again  on older macOs versions)
>>
>> That issue is being discussed elsewhere by the client folk and should
>> not be part of this change.
>>
>> David
>>
>>> Bug / webrev :
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8234741
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8234741.0/
>>>
>>> Thanks, Matthias
>>>