From d0bb7c137a00af182281e18194049c8f567ffb56 Mon Sep 17 00:00:00 2001 From: Ouss4 Date: Wed, 29 Apr 2020 17:53:36 +0100 Subject: [PATCH] Use NuttX's signal set functions inside the OS. --- drivers/modem/altair/altmdm_sys.c | 12 +++-- drivers/rptun/rptun.c | 2 +- include/nuttx/signal.h | 73 ++++++++++++++++++++++++++ libs/libc/signal/sig_addset.c | 55 +++++++++++++++---- libs/libc/signal/sig_delset.c | 55 +++++++++++++++---- libs/libc/signal/sig_ismember.c | 54 ++++++++++++++++--- sched/group/group_signal.c | 13 +++-- sched/sched/sched_waitid.c | 6 ++- sched/sched/sched_waitpid.c | 6 ++- sched/signal/sig_default.c | 12 ++--- sched/signal/sig_dispatch.c | 20 ++++--- sched/signal/sig_kill.c | 2 +- sched/signal/sig_lowest.c | 4 +- sched/signal/sig_pending.c | 3 +- sched/signal/sig_timedwait.c | 2 +- sched/signal/sig_unmaskpendingsignal.c | 4 +- sched/wqueue/kwork_process.c | 2 +- 17 files changed, 270 insertions(+), 55 deletions(-) diff --git a/drivers/modem/altair/altmdm_sys.c b/drivers/modem/altair/altmdm_sys.c index 3540941f8f9..fb973c934de 100644 --- a/drivers/modem/altair/altmdm_sys.c +++ b/drivers/modem/altair/altmdm_sys.c @@ -42,6 +42,8 @@ #include #include +#include + #include "altmdm_dev.h" #include "altmdm_sys.h" @@ -443,7 +445,8 @@ int altmdm_sys_waitflag(FAR struct altmdm_sys_flag_s *handle, } abs_time.tv_sec = timeout_ms / 1000; - abs_time.tv_nsec = (timeout_ms - (abs_time.tv_sec * 1000)) * 1000 * 1000; + abs_time.tv_nsec = (timeout_ms - (abs_time.tv_sec * 1000)) * + 1000 * 1000; abs_time.tv_sec += curr_time.tv_sec; abs_time.tv_nsec += curr_time.tv_nsec; @@ -557,7 +560,8 @@ int altmdm_sys_waitflag(FAR struct altmdm_sys_flag_s *handle, * ****************************************************************************/ -int altmdm_sys_setflag(FAR struct altmdm_sys_flag_s *handle, uint32_t pattern) +int altmdm_sys_setflag(FAR struct altmdm_sys_flag_s *handle, + uint32_t pattern) { int ret; irqstate_t flags; @@ -665,7 +669,7 @@ timer_t altmdm_sys_starttimer(int first_ms, int interval_ms, } sigemptyset(&mask); - sigaddset(&mask, MY_TIMER_SIGNAL); + nxsig_addset(&mask, MY_TIMER_SIGNAL); ret = sigprocmask(SIG_UNBLOCK, &mask, NULL); if (ret != OK) @@ -677,7 +681,7 @@ timer_t altmdm_sys_starttimer(int first_ms, int interval_ms, sa.sa_sigaction = handler; sa.sa_flags = SA_SIGINFO; sigfillset(&sa.sa_mask); - sigdelset(&sa.sa_mask, MY_TIMER_SIGNAL); + nxsig_delset(&sa.sa_mask, MY_TIMER_SIGNAL); ret = sigaction(MY_TIMER_SIGNAL, &sa, NULL); if (ret != OK) diff --git a/drivers/rptun/rptun.c b/drivers/rptun/rptun.c index c568077a993..0a8a2ffed3e 100644 --- a/drivers/rptun/rptun.c +++ b/drivers/rptun/rptun.c @@ -183,7 +183,7 @@ static int rptun_thread(int argc, FAR char *argv[]) priv = (FAR struct rptun_priv_s *)((uintptr_t)strtoul(argv[2], NULL, 0)); sigemptyset(&set); - sigaddset(&set, SIGUSR1); + nxsig_addset(&set, SIGUSR1); nxsig_procmask(SIG_BLOCK, &set, NULL); while (1) diff --git a/include/nuttx/signal.h b/include/nuttx/signal.h index 85f119bab0d..2a33f4dda2c 100644 --- a/include/nuttx/signal.h +++ b/include/nuttx/signal.h @@ -112,6 +112,79 @@ struct sigwork_s struct timespec; /* Forward reference */ +/**************************************************************************** + * Name: nxsig_ismember + * + * Description: + * This function tests whether the signal specified by signo is a member + * of the set specified by set. + * + * Input Parameters: + * set - Signal set to test + * signo - Signal to test for + * + * Returned Value: + * This is an internal OS interface and should not be used by applications. + * On success, it returns 0 if the signal is not a member, 1 if the signal + * is a member of the set. + * A negated errno value is returned on failure. + * + * EINVAL - The signo argument is invalid. + * + * Assumptions: + * + ****************************************************************************/ + +int nxsig_ismember(FAR const sigset_t *set, int signo); + +/**************************************************************************** + * Name: nxsig_addset + * + * Description: + * This function adds the signal specified by signo to the signal set + * specified by set. + * + * Input Parameters: + * set - Signal set to add signal to + * signo - Signal to add + * + * Returned Value: + * This is an internal OS interface and should not be used by applications. + * It follows the NuttX internal error return policy: Zero (OK) is + * returned on success. A negated errno value is returned on failure. + * + * EINVAL - The signo argument is invalid. + * + * Assumptions: + * + ****************************************************************************/ + +int nxsig_addset(FAR sigset_t *set, int signo); + +/**************************************************************************** + * Name: nxsig_delset + * + * Description: + * This function deletes the signal specified by signo from the signal + * set specified by the 'set' argument. + * + * Input Parameters: + * set - Signal set to delete the signal from + * signo - Signal to delete + * + * Returned Value: + * This is an internal OS interface and should not be used by applications. + * It follows the NuttX internal error return policy: Zero (OK) is + * returned on success. A negated errno value is returned on failure. + * + * EINVAL - The signo argument is invalid. + * + * Assumptions: + * + ****************************************************************************/ + +int nxsig_delset(FAR sigset_t *set, int signo); + /**************************************************************************** * Name: nxsig_procmask * diff --git a/libs/libc/signal/sig_addset.c b/libs/libc/signal/sig_addset.c index 17891c32c75..ac8d095a689 100644 --- a/libs/libc/signal/sig_addset.c +++ b/libs/libc/signal/sig_addset.c @@ -44,6 +44,45 @@ * Public Functions ****************************************************************************/ +/**************************************************************************** + * Name: nxsig_addset + * + * Description: + * This function adds the signal specified by signo to the signal set + * specified by set. + * + * Input Parameters: + * set - Signal set to add signal to + * signo - Signal to add + * + * Returned Value: + * This is an internal OS interface and should not be used by applications. + * It follows the NuttX internal error return policy: Zero (OK) is + * returned on success. A negated errno value is returned on failure. + * + * EINVAL - The signo argument is invalid. + * + * Assumptions: + * + ****************************************************************************/ + +int nxsig_addset(FAR sigset_t *set, int signo) +{ + /* Verify the signal */ + + if (!GOOD_SIGNO(signo)) + { + return -EINVAL; + } + else + { + /* Add the signal to the set */ + + *set |= SIGNO2SET(signo); + return OK; + } +} + /**************************************************************************** * Name: sigaddset * @@ -64,18 +103,16 @@ int sigaddset(FAR sigset_t *set, int signo) { - /* Verify the signal */ + int ret; - if (!GOOD_SIGNO(signo)) + /* Let nxsig_addset do all the work. */ + + ret = nxsig_addset(set, signo); + if (ret < 0) { set_errno(EINVAL); - return ERROR; + ret = ERROR; } - else - { - /* Add the signal to the set */ - *set |= SIGNO2SET(signo); - return OK; - } + return ret; } diff --git a/libs/libc/signal/sig_delset.c b/libs/libc/signal/sig_delset.c index 6fdeb7cd046..72a2b05b7cb 100644 --- a/libs/libc/signal/sig_delset.c +++ b/libs/libc/signal/sig_delset.c @@ -44,6 +44,45 @@ * Public Functions ****************************************************************************/ +/**************************************************************************** + * Name: nxsig_delset + * + * Description: + * This function deletes the signal specified by signo from the signal + * set specified by the 'set' argument. + * + * Input Parameters: + * set - Signal set to delete the signal from + * signo - Signal to delete + * + * Returned Value: + * This is an internal OS interface and should not be used by applications. + * It follows the NuttX internal error return policy: Zero (OK) is + * returned on success. A negated errno value is returned on failure. + * + * EINVAL - The signo argument is invalid. + * + * Assumptions: + * + ****************************************************************************/ + +int nxsig_delset(FAR sigset_t *set, int signo) +{ + /* Verify the signal */ + + if (!GOOD_SIGNO(signo)) + { + return -EINVAL; + } + else + { + /* Remove the signal from the set */ + + *set &= ~SIGNO2SET(signo); + return OK; + } +} + /**************************************************************************** * Name: sigdelset * @@ -64,18 +103,16 @@ int sigdelset(FAR sigset_t *set, int signo) { - /* Verify the signal */ + int ret; - if (!GOOD_SIGNO(signo)) + /* Let nxseg_delset do all the work. */ + + ret = nxsig_delset(set, signo); + if (ret < 0) { set_errno(EINVAL); - return ERROR; + ret = ERROR; } - else - { - /* Remove the signal from the set */ - *set &= ~SIGNO2SET(signo); - return OK; - } + return ret; } diff --git a/libs/libc/signal/sig_ismember.c b/libs/libc/signal/sig_ismember.c index 443404a771e..39ec44b911f 100644 --- a/libs/libc/signal/sig_ismember.c +++ b/libs/libc/signal/sig_ismember.c @@ -38,11 +38,53 @@ ****************************************************************************/ #include +#include + +#include /**************************************************************************** * Public Functions ****************************************************************************/ +/**************************************************************************** + * Name: nxsig_ismember + * + * Description: + * This function tests whether the signal specified by signo is a member + * of the set specified by set. + * + * Input Parameters: + * set - Signal set to test + * signo - Signal to test for + * + * Returned Value: + * This is an internal OS interface and should not be used by applications. + * On success, it returns 0 if the signal is not a member, 1 if the signal + * is a member of the set. + * A negated errno value is returned on failure. + * + * EINVAL - The signo argument is invalid. + * + * Assumptions: + * + ****************************************************************************/ + +int nxsig_ismember(FAR const sigset_t *set, int signo) +{ + /* Verify the signal */ + + if (!GOOD_SIGNO(signo)) + { + return -EINVAL; + } + else + { + /* Check if the signal is in the set */ + + return ((*set & SIGNO2SET(signo)) != 0); + } +} + /**************************************************************************** * Name: sigismember * @@ -65,15 +107,15 @@ int sigismember(FAR const sigset_t *set, int signo) { - int ret = ERROR; + int ret; - /* Verify the signal */ + /* Let nxsig_ismember do all of the work */ - if (GOOD_SIGNO(signo)) + ret = nxsig_ismember(set, signo); + if (ret < 0) { - /* Check if the signal is in the set */ - - ret = ((*set & SIGNO2SET(signo)) != 0); + set_errno(EINVAL); + ret = ERROR; } return ret; diff --git a/sched/group/group_signal.c b/sched/group/group_signal.c index 757fdd79bf8..a2198552bc3 100644 --- a/sched/group/group_signal.c +++ b/sched/group/group_signal.c @@ -45,6 +45,8 @@ #include #include +#include + #include "sched/sched.h" #include "group/group.h" #include "signal/signal.h" @@ -98,7 +100,9 @@ static int group_signal_handler(pid_t pid, FAR void *arg) if (tcb) { - /* Set this one as the default if we have not already set the default. */ + /* Set this one as the default if we have not already set the + * default. + */ if (!info->dtcb) { @@ -109,7 +113,8 @@ static int group_signal_handler(pid_t pid, FAR void *arg) * probably blocked). */ - if (sigismember(&tcb->sigwaitmask, info->siginfo->si_signo) && !info->atcb) + ret = nxsig_ismember(&tcb->sigwaitmask, info->siginfo->si_signo); + if (ret == 1 && !info->atcb) { /* Yes.. This means that the task is suspended, waiting for this * signal to occur. Stop looking and use this TCB. The @@ -136,7 +141,7 @@ static int group_signal_handler(pid_t pid, FAR void *arg) /* Is this signal unblocked on this thread? */ - if (!sigismember(&tcb->sigprocmask, info->siginfo->si_signo) && + if (!nxsig_ismember(&tcb->sigprocmask, info->siginfo->si_signo) && !info->ptcb && tcb != info->atcb) { /* Yes.. remember this TCB if we have not encountered any @@ -197,7 +202,7 @@ static int group_signal_handler(pid_t pid, FAR void *arg) * 0 (OK) on success; a negated errno value on failure. * * Assumptions: - * Called during task termination in a safe context. No special precautions + * Called during task termination in a safe context. No special precautions * are required here. Because signals can be sent from interrupt handlers, * this function may be called indirectly in the context of an interrupt * handler. diff --git a/sched/sched/sched_waitid.c b/sched/sched/sched_waitid.c index b70caad380a..a3431dec665 100644 --- a/sched/sched/sched_waitid.c +++ b/sched/sched/sched_waitid.c @@ -202,7 +202,7 @@ int waitid(idtype_t idtype, id_t id, FAR siginfo_t *info, int options) /* Create a signal set that contains only SIGCHLD */ sigemptyset(&set); - sigaddset(&set, SIGCHLD); + nxsig_addset(&set, SIGCHLD); /* Disable pre-emption so that nothing changes while the loop executes */ @@ -314,7 +314,9 @@ int waitid(idtype_t idtype, id_t id, FAR siginfo_t *info, int options) } } - /* We are waiting for a specific PID. Does this task retain child status? */ + /* We are waiting for a specific PID. Does this task retain child + * status? + */ else if (retains) { diff --git a/sched/sched/sched_waitpid.c b/sched/sched/sched_waitpid.c index 644249d42a2..8ff52448dc8 100644 --- a/sched/sched/sched_waitpid.c +++ b/sched/sched/sched_waitpid.c @@ -331,7 +331,7 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options) /* Create a signal set that contains only SIGCHLD */ sigemptyset(&set); - sigaddset(&set, SIGCHLD); + nxsig_addset(&set, SIGCHLD); /* Disable pre-emption so that nothing changes while the loop executes */ @@ -455,7 +455,9 @@ pid_t waitpid(pid_t pid, int *stat_loc, int options) } } - /* We are waiting for a specific PID. Does this task retain child status? */ + /* We are waiting for a specific PID. Does this task retain child + * status? + */ else if (retains) { diff --git a/sched/signal/sig_default.c b/sched/signal/sig_default.c index 719adc900b2..b30a7daf83e 100644 --- a/sched/signal/sig_default.c +++ b/sched/signal/sig_default.c @@ -459,7 +459,7 @@ static void nxsig_setup_default_action(FAR struct task_group_s *group, /* Indicate that the default signal handler has been attached */ - sigaddset(&group->tg_sigdefault, (int)info->signo); + nxsig_addset(&group->tg_sigdefault, (int)info->signo); } } @@ -495,7 +495,7 @@ bool nxsig_isdefault(FAR struct tcb_s *tcb, int signo) * false in all other cases. */ - ret = sigismember(&group->tg_sigdefault, signo); + ret = nxsig_ismember(&group->tg_sigdefault, signo); return ret < 0 ? false : (bool)ret; } @@ -579,22 +579,22 @@ _sa_handler_t nxsig_default(FAR struct tcb_s *tcb, int signo, bool defaction) handler = nxsig_default_action(signo); if (handler != SIG_IGN) { - /* sigaddset() is not atomic (but neither is sigaction()) */ + /* nxsig_addset() is not atomic (but neither is sigaction()) */ flags = spin_lock_irqsave(); - sigaddset(&group->tg_sigdefault, signo); + nxsig_addset(&group->tg_sigdefault, signo); spin_unlock_irqrestore(flags); } } if (handler == SIG_IGN) { - /* We are unsetting the default action. NOTE that sigdelset() is not + /* We are unsetting the default action. NOTE that nxsig_delset() is not * atomic (but neither is sigaction()). */ flags = spin_lock_irqsave(); - sigdelset(&group->tg_sigdefault, signo); + nxsig_delset(&group->tg_sigdefault, signo); spin_unlock_irqrestore(flags); } diff --git a/sched/signal/sig_dispatch.c b/sched/signal/sig_dispatch.c index 013d1440c8a..80776ea3606 100644 --- a/sched/signal/sig_dispatch.c +++ b/sched/signal/sig_dispatch.c @@ -49,6 +49,7 @@ #include #include +#include #include "sched/sched.h" #include "group/group.h" @@ -301,7 +302,7 @@ static void nxsig_add_pendingsignal(FAR struct tcb_s *stcb, int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info) { irqstate_t flags; - bool masked; + int masked; int ret = OK; sinfo("TCB=0x%08x signo=%d code=%d value=%d mask=%08x\n", @@ -319,7 +320,7 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info) /************************** MASKED SIGNAL ACTIONS *************************/ - masked = (bool)sigismember(&stcb->sigprocmask, info->si_signo); + masked = nxsig_ismember(&stcb->sigprocmask, info->si_signo); #ifdef CONFIG_LIB_SYSCALL /* Check if the signal is masked OR if the signal is received while we are @@ -346,13 +347,13 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info) * signal handlers. */ - if (masked || (stcb->flags & TCB_FLAG_SYSCALL) != 0) + if ((masked == 1) || (stcb->flags & TCB_FLAG_SYSCALL) != 0) #else /* Check if the signal is masked. In that case, it will be added to the * list of pending signals. */ - if (masked) + if (masked == 1) #endif { /* Check if the task is waiting for this pending signal. If so, then @@ -362,7 +363,7 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info) flags = enter_critical_section(); if (stcb->task_state == TSTATE_WAIT_SIG && - sigismember(&stcb->sigwaitmask, info->si_signo)) + nxsig_ismember(&stcb->sigwaitmask, info->si_signo)) { memcpy(&stcb->sigunbinfo, info, sizeof(siginfo_t)); stcb->sigwaitmask = NULL_SIGNAL_SET; @@ -426,7 +427,7 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info) * happen within a system call. */ - if (!masked) + if (masked == 0) { /* If the task is blocked waiting for a semaphore, then that task must * be unblocked when a signal is received. @@ -466,6 +467,13 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info) #endif } + /* In case nxsig_ismember failed due to an invalid signal number */ + + if (masked < 0) + { + ret = -EINVAL; + } + return ret; } diff --git a/sched/signal/sig_kill.c b/sched/signal/sig_kill.c index dcf4b5b80b0..d8e75876e1d 100644 --- a/sched/signal/sig_kill.c +++ b/sched/signal/sig_kill.c @@ -167,7 +167,7 @@ int kill(pid_t pid, int signo) { int ret; - /* Let nxsem_kill() do all of the work */ + /* Let nxsig_kill() do all of the work */ ret = nxsig_kill(pid, signo); if (ret < 0) diff --git a/sched/signal/sig_lowest.c b/sched/signal/sig_lowest.c index 63cc42cedaf..2c82b918e73 100644 --- a/sched/signal/sig_lowest.c +++ b/sched/signal/sig_lowest.c @@ -41,6 +41,8 @@ #include +#include + #include "signal/signal.h" /**************************************************************************** @@ -61,7 +63,7 @@ int nxsig_lowest(sigset_t *set) for (signo = MIN_SIGNO; signo <= MAX_SIGNO; signo++) { - if (sigismember(set, signo)) + if (nxsig_ismember(set, signo)) { return signo; } diff --git a/sched/signal/sig_pending.c b/sched/signal/sig_pending.c index 103a83488e7..b5309c563e5 100644 --- a/sched/signal/sig_pending.c +++ b/sched/signal/sig_pending.c @@ -43,6 +43,7 @@ #include #include +#include #include "sched/sched.h" #include "signal/signal.h" @@ -106,7 +107,7 @@ sigset_t nxsig_pendingset(FAR struct tcb_s *stcb) for (sigpend = (FAR sigpendq_t *)group->tg_sigpendingq.head; (sigpend); sigpend = sigpend->flink) { - sigaddset(&sigpendset, sigpend->info.si_signo); + nxsig_addset(&sigpendset, sigpend->info.si_signo); } leave_critical_section(flags); diff --git a/sched/signal/sig_timedwait.c b/sched/signal/sig_timedwait.c index ad75792c285..9d873464304 100644 --- a/sched/signal/sig_timedwait.c +++ b/sched/signal/sig_timedwait.c @@ -406,7 +406,7 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info, * that we were waiting for? */ - if (sigismember(set, rtcb->sigunbinfo.si_signo)) + if (nxsig_ismember(set, rtcb->sigunbinfo.si_signo)) { /* Yes.. the return value is the number of the signal that * awakened us. diff --git a/sched/signal/sig_unmaskpendingsignal.c b/sched/signal/sig_unmaskpendingsignal.c index 7ea803b4e06..cff799ec682 100644 --- a/sched/signal/sig_unmaskpendingsignal.c +++ b/sched/signal/sig_unmaskpendingsignal.c @@ -41,6 +41,8 @@ #include +#include + #include "sched/sched.h" #include "signal/signal.h" @@ -101,7 +103,7 @@ bool nxsig_unmask_pendingsignal(void) * signal number is pending. */ - sigdelset(&unmaskedset, signo); + nxsig_delset(&unmaskedset, signo); /* Remove the pending signal from the list of pending signals */ diff --git a/sched/wqueue/kwork_process.c b/sched/wqueue/kwork_process.c index 61eeeb38d91..2be9edf6b13 100644 --- a/sched/wqueue/kwork_process.c +++ b/sched/wqueue/kwork_process.c @@ -222,7 +222,7 @@ void work_process(FAR struct kwork_wqueue_s *wqueue, int wndx) /* Wait indefinitely until signalled with SIGWORK */ sigemptyset(&set); - sigaddset(&set, SIGWORK); + nxsig_addset(&set, SIGWORK); wqueue->worker[wndx].busy = false; DEBUGVERIFY(nxsig_waitinfo(&set, NULL));