Don't actually send a signal with signo 0

[yamt]

As stated by standards.

[patacongo]

This should not be needed. The check for signal zero is handled much high up in the signal sequence. Signal 0 should never make it to this code.

Are you seeing an actual bug that this fixes? Otherwise, I believe that this change is incorrect and I will decline it.

[patacongo]

Are you seeing an actual bug that this fixes?

If you are, I would also like to know the logic sequence. As I said, signal 0 should never get here. A DEBUGASSERT(signal != 0) might be appropriate.

[yamt]

@patacongo
yes, i saw the symptom.
can you pinpoint where "much high up in the signal sequence" is?

(gdb) bt
#0 nxsig_tcbdispatch (stcb=0xfffffffd, info=0x3ffb7b34)
at signal/sig_dispatch.c:302
#1 0x400d19e0 in nxsig_dispatch (pid=, info=0x3ffb7b34)
at signal/sig_dispatch.c:546
#2 0x400dc028 in nxsig_kill (pid=5, signo=0) at signal/sig_kill.c:129
#3 0x400dc03c in kill (pid=5, signo=0) at signal/sig_kill.c:172

[patacongo]

Then that is an error in kill() (or really nxsig_kill()). kill() has specific requirements to ignore signal 0 https://pubs.opengroup.org/onlinepubs/009695399/functions/kill.html

That change should go in kill(), not nxsig_dispatch().

[yamt]

@patacongo
i disagree.

• kill(pid, 0) still needs to do error checks (ESRCH) which is done by nxsig_dispatch.
• the signo=0 semantics are shard with other API like sigqueue and pthread_kill.

[patacongo]

I don't see the logic in kill(), it should be there because this feature has been used for years, so I am surprised.

But I still need to close this change. It is not correct. Any special signal handling must be moved higher. The logic in nxsig_dispatch may do other, improper things with signal 0. It may for example, call group_signal() instead of nxsig_tcbdispatch().

The specific requirement for signal zero handler ONLY applies to kill() and sigqueue() so those would be the best place in order to keep the requirement associated with the interface. But the check the existence of the task is not performed until nxsid_tcbdispatch() so maybe that would be an acceptable place. I don't think nxsig_tcbdispatch() is the correct place.

[patacongo]

Basically, your change only works if HAVE_GROUP_MEMBERS is not defined. If that is defined, then calling kill() or sigqueue() will go a different path:

kill->nxsig_kill->nxsig_dispatch->group_signal

You change also does not handle multi-threaded task groups. The task ID may no longer be valid, but the task group may persist. In this case, a different pthread member of the group will receive the signal.

That can be fixed by moving the null signal check to lines 528 and 546

[yamt]

group_signal uses nxsig_tcbdispatch. it was actually one of the reasons why i fixed it this way.

[yamt]

do you have an example of user-facinig API with a different semantics for signo zero?

[patacongo]

There are three ways to get to nxsig_dispatch(): kill(), sigqueue(), and via signal notifications which are used message queue and timer signal notifications. Those all seem okay. The only problem is at nxsig_dispatch() which can have different behaviors depending on configuration. So any zero signal check needs to go early in nxsg_dispatch() while the common patch is being followed.

[patacongo]

Okay, it sounds like you have done a complete analysis. I will merge the change.