patacongo commented on code in PR #9848:
URL: https://github.com/apache/nuttx/pull/9848#discussion_r1267507292


##########
sched/signal/sig_nanosleep.c:
##########
@@ -261,6 +262,11 @@ int clock_nanosleep(clockid_t clockid, int flags,
 {
   int ret;
 
+  if (clockid < CLOCK_REALTIME || clockid > CLOCK_BOOTTIME)
+    {
+      return EINVAL;
+    }

Review Comment:
   > @xiaoxiang781216 @patacongo do you know what should have more "power" 
here, cancelation point or invalid parameter check? In other words, should 
these lines be placed here or after `enter_cancellation_point();`? Or in other 
words, should `clock_nanosleep()` call with invalid `clockid` be equivalent to 
`pthread_testcancel` or not?
   
   enter_cancellation_point() catches the case where the thread is canceled 
prior to calling the cancellation point function.  enter_cancellation_point() 
does not return if there is a pending cancellation and the cancellation is not 
nested; it just exits.  The the cancellation point is nested, then 
enter_cancellation_point() just increments cpcount and returns true. 
   
   I would think that exiting would be more important than returning an error 
in that case.  Otherwise the thread may not exit until the next time a 
cancellation point function is called.  That would probably actually violate 
the POSIX requirement.
   
   The only issue that I see is that enter_cancellation_point() increments 
cpcount.  Returning without calling leave_cancellation_point() will leave 
cpcount incremented meaning that we are still in a (possibly nested) 
cancellation point.  That would need to be fixed up somewhere otherwise, moving 
the EINVAL return after enter_critical_secction() would mess up the 
cancellation point logic.
   
   This is usually handled like this in functions like sem_wait():
   
         ret = nxsem_wait(sem);
         if (ret < 0)
           {
             errcode = -ret;
             goto errout_with_cancelpt;
           }
       ...
       errout_with_cancelpt:
         set_errno(errcode);
         leave_cancellation_point();
         return ERROR;



-- 
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-unsubscr...@nuttx.apache.org

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

Reply via email to