signal: Add SIG_FOREACH and refactor issignal()
ClosedPublic
Actions

Authored by markj on Oct 12 2021, 9:10 PM.

Details

Reviewers

Commits

rG81f2e9063d64: signal: Add SIG_FOREACH and refactor issignal()

Summary

Add a SIG_FOREACH macro that can be used to iterate over a signal set.
This is a bit cleaner and more efficient than calling sig_ffs() in a
loop. The implementation is based on BIT_FOREACH_ISSET(), except
that the bitset limbs are always 32 bits wide, and signal sets are
1-indexed rather than 0-indexed like bitset(9) sets.

issignal() cannot really be modified to use SIG_FOREACH() directly.
Take this opportunity to split the function into two explicit loops.
I've always found this function hard to read and think that this change
is an improvement. The inner body of issignal() is unchanged, except
that sigprop() is only called when needed now.

Remove sig_ffs(), nothing uses it now.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 42094
Build 38982: arc lint + arc unit

Event Timeline

markj created this revision.Oct 12 2021, 9:10 PM

Herald added a subscriber: imp. · View Herald TranscriptOct 12 2021, 9:10 PM

markj requested review of this revision.Oct 12 2021, 9:10 PM

Harbormaster completed remote builds in B42093: Diff 96766.Oct 12 2021, 9:10 PM

kib added inline comments.Oct 13 2021, 12:21 AM

sys/kern/kern_sig.c
3157	Isn't break equivalent to goto next there. Then this switch is same as the switch inside P_TRACED block above. Then, could you replace sigpending with a mask consisting only of SIGSTOP and use one switch?

markj added inline comments.Oct 13 2021, 12:48 AM

sys/kern/kern_sig.c
3157	Oops, I had such a change stashed and forgot to add it. Yes, it is possible to merge the switch statements.

Merge switch statements.

Harbormaster completed remote builds in B42094: Diff 96772.Oct 13 2021, 12:49 AM

kib added inline comments.Oct 13 2021, 12:58 AM

sys/kern/kern_sig.c
3157	Still please either use break in HANDLED case, or goto next in NEXT.

markj added inline comments.Oct 13 2021, 1:22 AM

sys/kern/kern_sig.c
3157	break and goto next are not equivalent here. goto next reloads the sigpending set from the thread+process. "break" continues iteration within the already loaded sigpending set. SIGSTATUS_NEXT is returned only when nothing was done and the proc lock was not dropped.

kib added inline comments.Oct 13 2021, 10:34 AM

sys/kern/kern_sig.c
3084	I suggest to move both sigqueue_delete calls to getsignal(), and replace all 'goto next' in this function by return (SIGSTATUS_NEXT);. Also I think that SIGSTATUS_NEXT is better named SIGSTATUS_IGNORE
3100	Why not call this function issignal? Then you do not need to change cursig() and it makes one less thing to track between older and newer branches.