RFR (S): 7129724: Fix location of core file in dump message on macosx
David Holmes
david.holmes at oracle.com
Tue Jun 26 17:47:20 PDT 2012
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
>> ----
>
>
More information about the hotspot-runtime-dev
mailing list