RFR: JDK-8260485: Simplify and unify handler vectors in Posix signal code [v6]

Gerard Ziemski gziemski at openjdk.java.net
Wed Feb 17 16:13:43 UTC 2021


On Wed, 17 Feb 2021 05:42:11 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> I would like to keep this. SavedSignalHandlers::check_signal_number() is just a simple bounds check to prevent memory overwriters, with an added assert for goot measure in debug builds. Its proximity to the array it guards makes this clear. SavedSignalHandlers gets only fed with know values in this file, so an assert is sufficient.
>> 
>> is_valid_signal() OTOH does a runtime check for signal validity. It is more expensive but only used in analysis situations. It pulls its signal number from a sigaction structure, which may be corrupt and contain whatever, so the runtime check makes more sense to me.
>
> In fact, SavedSignalHandler::_sa[NSIG] could be reduced in size to something more sensible, since we only ever feed "normal" signals into it. Same reason as NUM_IMPORTANT_SIGS. But we only talk a few kbyte here, so I did not bother.

I would have preferred to have a single function to check whether the signal is valid, otherwise we have 2 separate specialized API that almost do the same thing.  Any future reader of the code will probably wonder again.

For code readability I'd prefer to have just one API, or at least refactor the more complex one in terms of the simpler one, and give them meaningful names to differentiate them, but OK.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2251


More information about the hotspot-dev mailing list