RFR (S): 7129724: Fix location of core file in dump message on macosx

[Mikael Vidstedt]

David,

You're right. I was set on maintaining the "correct" string, but 
relaxing that somewhat does make it easier.

http://cr.openjdk.java.net/~mikael/7129724/webrev.02

Thanks,
Mikael

On 6/26/2012 5:47 PM, David Holmes wrote:
> Hi Mikael,
>
> On 27/06/2012 10:31 AM, Mikael Vidstedt wrote:
>>
>> David,
>>
>> Do you mean something like:
>>
>> http://cr.openjdk.java.net/~mikael/7129724/webrev.01
>
> Something like, but to simplify things I was thinking of just 
> returning the actual path component and have the check_or_create_dump 
> handle the "core or core.%d" part. No need to duplicate the pid 
> parsing logic then and the main part for linux/solaris is simply:
>
>  int os::get_core_paths(char* buffer, size_t bufferSize) {
>    get_current_directory(buffer, bufferSize);
>    return strlen(buffer);
>  }
>
> (this is unchecked - I'm assuming things are properly nul-terminated 
> etc. Pity about the need for strlen, but then you could choose not to 
> return the length and instead do the strlen directly ie:
>
> !     os::get_core_paths(buffer, bufferSize);
>       n = strlen(buffer);
> !     jio_snprintf(buffer + n, bufferSize - n, "...");
>
> either way works I guess)
>
> My suggestion does mean that the message is not as specific as it 
> might be if the name is always core.pid rather than core. But that 
> seems a very minor concern.
>
> BTW in current webrev this part needs fixing up:
>
>     if (getrlimit(RLIMIT_CORE, &rlim) != 0) {
> !     n = os::get_core_paths(buffer, bufferSize);
> !     jio_snprintf(buffer + n, bufferSize - n, "%s/core or core.%d 
> (may not exist)");
>       success = true;
>
> You are no longer parsing anything for the %s and %d - I think you 
> really only want the "may not exist" part.
>
> Thanks,
> David
>
>> Cheers,
>> Mikael
>>
>> On 2012-06-25 23:31, David Holmes wrote:
>>> Hi Mikael,
>>>
>>> On 26/06/2012 8:22 AM, Mikael Vidstedt wrote:
>>>> If HotSpot crashes the dump message prints the default location of the
>>>> generated core file. On other "posix" like OSes the path is normally
>>>> <cwd>/core or core.<pid>, but on macosx the cores always end up in
>>>> /cores/core.<pid>. This change aims to reflect that. Verified manually
>>>> on Linux and macosx. Passes JPRT.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~mikael/7129724/webrev.00
>>>
>>> I don't like this - sorry. The whole point of introducing
>>> os_posix.cpp/hpp was to capture common "posix" functionality. The very
>>> fact you need to add this change indicates this is not in fact "common
>>> posix functionality". Seems to me that the default core location
>>> should be a platform specific value:
>>>
>>> os::get_corefile_path(char* buf, int buflen)
>>>
>>> and then each os_*.cpp can define it as needed, with Solaris and Linux
>>> simply delegating to the existing os::get_current_directory(buf, len).
>>>
>>> That aside, it really bugs me that BSD/OSX things are conditionalized
>>> on __APPLE__ ! Should be BSD or OSX or our internally defined values
>>> like TARGET_OS_FAMILY_xxx
>>>
>>> Cheers,
>>> David
>>> ----
>>
>>