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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
