RFR: JDK-8258606: os::print_signal_handlers() should resolve the function name of the handlers [v2]

[David Holmes]

Hi Thomas,

Sorry for the long delay getting back to this.

On 28/12/2020 5:52 pm, Thomas Stuefe wrote:
> On Sat, 26 Dec 2020 09:44:22 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Feedback Coleen
>>
>> src/hotspot/share/runtime/os.hpp line 618:
>>
>>> 616:                                               bool shorten_paths = true,
>>> 617:                                               bool demangle = true,
>>> 618:                                               bool strip_arguments = false);
>>
>> Are you actually relying on these having default values now?
> 
> I think it makes sense. The scratch buffer arguments are useful during error handling but a bit cumbersome in normal situations. Since I want this to be a general purpose function, I make the scratch buffer optional.
> 
> Btw, do you object against the `alloca` in the implementation? We use it in some few other places too. But it was a coin toss really whether to use (raw) malloc instead.

As you say it is a coin-toss. If we're processing a stackoverflow then 
IIUC the guard pages should now be disabled as we "crash" and so we 
should have room for the alloca. If we use malloc then we risk the usual 
malloc reentrancy problems. So I'm fine with alloca.

>> src/hotspot/os/posix/signals_posix.cpp line 811:
>>
>>> 809:     os::print_function_and_library_name(tty, jvmHandler, buf, O_BUFLEN, true, true, true);
>>> 810:     tty->print_raw("  found:");
>>> 811:     os::print_function_and_library_name(tty, thisHandler, buf, O_BUFLEN, true, true, true);
>>
>> These would benefit from using the same style as line 1361. No-one will remember what the three boolean flags represent.
> 
> You are right. I reworked the style.

Thanks that looks much simpler.

David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/1839
>