RFR: JDK-8260485: Simplify and unify handler vectors in Posix signal code [v6]
Thomas Stuefe
stuefe at openjdk.java.net
Wed Feb 17 05:56:45 UTC 2021
On Mon, 15 Feb 2021 22:35:30 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:
>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Use universal zero initializer for do_check_signal_periodically
>
> src/hotspot/os/posix/signals_posix.cpp line 414:
>
>> 412: if (actp == NULL) {
>> 413: // Retrieve the preinstalled signal handler from jvm
>> 414: actp = const_cast<struct sigaction*>(chained_handlers.get(sig));
>
> Must `SavedSignalHandlers::get()` have the **const struct** in `const struct sigaction* get(int sig) const` signature?
>
> If it was just `struct sigaction* get(int sig) const` then we wouldn't need this awkward cast.
I'm a stickler to const since its very expressive and prevents stupid errors. `const struct sigaction* get() const` makes perfect sense since at no time you are supposed to modify the underlying sigaction structure. Once it is saved, it should only be used to read from (eg to print, or to extract the handler address and call it in `call_chained_handler()`.
But old code was not const correct. The way to start being const correct without having to rewrite everything is to make new code const correct and cauterize at the interface between new and old code with these ugly but at least expressive casts.
Side note, even the sigaction function is const correct: https://pubs.opengroup.org/onlinepubs/007904875/functions/sigaction.html
see the `act` input parameter designating the new handler, its const. Only the output parameter `oact` is nonconst.
I will see if I can make the rest of the code const correct too wrt the sigaction structures. That way we can get rid of the cast and have better code as well.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2251
More information about the hotspot-dev
mailing list