RFR: 8253742: POSIX signal code cleanup [v3]

[David Holmes]

On 7/11/2020 2:40 am, Gerard Ziemski wrote:
> On Wed, 4 Nov 2020 04:22:05 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Gerard Ziemski has updated the pull request incrementally with two additional commits since the last revision:
>>>
>>>   - use ifdef(SIGDANGER) and ifdef(SIGTRAP)
>>>   - revert unblock_program_error_signals change
>>
>> src/hotspot/os/posix/signals_posix.hpp line 33:
>>
>>> 31:
>>> 32: typedef siginfo_t siginfo_t;
>>> 33: typedef sigset_t sigset_t;
>>
>> I don't see why this is needed/wanted. We can include signal.h without a problem.
>>
>> I'm not even sure what these typedefs means ??
> 
> Coleen asked me to remove <signal.h> from the signals_posix.hpp, so those are forward declarations for the signal types we use.
> 
> I thought it was a reasonable request to minimize the number of headers. I saw some efforts in the past to cleanup header files, which is supposed to help with build times, so every little bit helps.

The only reason we would care is if signals_posix.hpp were included in 
many other handers/files and that should not be the case. This looks 
completely bogus to me as we need the types from signal.h in this header 
file. What do those typedefs even mean? I would expect a forward 
declaration to be of the form:

struct siginfo_t;

but you don't know what type sigset_t (could be integer or struct) 
actually is so you can't forward declare it that way.

David
-----

>> src/hotspot/os/posix/signals_posix.cpp line 1642:
>>
>>> 1640:
>>> 1641: void PosixSignals::do_task(Thread* thread, os::SuspendedThreadTask* task) {
>>> 1642:   if (PosixSignals::do_suspend(thread->osthread())) {
>>
>> Shouldn't need PosixSignals:: prefix in this method.
> 
> Leftover from the time when do_task wasn't in signals_posix.cpp, fixed.
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/636
>