[GitHub] [nuttx] pussuw commented on a diff in pull request #8740: signal/sig_dispatch: Fix sending the same signal twice

["pussuw (via GitHub)" <gi...@apache.org>]

pussuw commented on code in PR #8740:
URL: https://github.com/apache/nuttx/pull/8740#discussion_r1126746251


##########
sched/signal/sig_dispatch.c:
##########
@@ -373,11 +373,11 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t *info)
 #endif
     {
 #ifdef CONFIG_LIB_SYSCALL
-      /* If the thread is in syscall, schedule the sigaction here */
+      /* If the thread is in syscall, queue the sigaction here */
 
-      if ((stcb->flags & TCB_FLAG_SYSCALL) != 0)
+      if (masked == 0)

Review Comment:
   I don't think that would be logically equivalent.
   
   The first if has really only one significant test when a signal is received during as system call:
   
   ```
   if (stcb->task_state == TSTATE_WAIT_SIG &&
             (masked == 0 || nxsig_ismember(&stcb->sigwaitmask, info->si_signo)))
   ```
   
   If masked == 0 and `stcb->task_state == TSTATE_WAIT_SIG` is true, the second branch will not be executed. So this has to be done either here, or moved inside the first if. I think this is better as we don't do the work of `nxsig_queue_action` inside a critical section.
   
   Also, the else-branch must test for masked==1, otherwise the signal is delivered twice (again). I agree this is a bit messy but I cannot see a better way to do this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org