You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/05/27 14:37:48 UTC
[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request, #6338: sched/semaphore: sem_wait should not report ECANCELED
pkarashchenko opened a new pull request, #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338
## Summary
`sem_wait` should not report `ECANCELED`
This is the first step to remove nested cancellation points support because nested cancellation point situation should never happen
## Impact
Should be none
## Testing
Pass CI
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883735242
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
Yeah, sorry not fully true:
```
static int uart_tcdrain(FAR uart_dev_t *dev,
bool cancelable, clock_t timeout)
{
int ret;
/* tcdrain is a cancellation point */
if (cancelable && enter_cancellation_point())
{
#ifdef CONFIG_CANCELLATION_POINTS
/* If there is a pending cancellation, then do not perform
* the wait. Exit now with ECANCELED.
*/
leave_cancellation_point();
return -ECANCELED;
#endif
}
```
also has it
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883729272
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
My guess is that this same pattern exists in all cancellation point functions after enter_cancellation_point() is called. Shouldn't they all be changed?
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883710600
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
Review Comment:
I think that this has an unacceptable side effect. In the deferred cancellation mode, there could be a cancellation pending on entry into sem_wait().
POSIX required that the deferred cancellation pend until a cancellation point function is called. But with this change, sem_wait() will ignore the deferred cancelation and just continue to wait.
The result would be that the cancellation is lost.
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched: remove nested cancelation point support
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r889366979
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
Done. Changes are still under test, but general concept is implemented. Please check
--
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
[GitHub] [incubator-nuttx] acassis commented on pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#issuecomment-1139718156
@pkarashchenko I suggest you to change it to Draft to further discussion
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883736115
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
Review Comment:
Oops... I didn't look carefully enough. Sorry. Looks fine.
--
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
Re: [PR] sched: remove nested cancelation point support [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #6338:
URL: https://github.com/apache/nuttx/pull/6338#issuecomment-3315988367
Is there anything that can be done with this PR or may I close it?
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883734112
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
For example this is clock wait:
```
int sem_clockwait(FAR sem_t *sem, clockid_t clockid,
FAR const struct timespec *abstime)
{
int ret;
/* sem_timedwait() is a cancellation point */
enter_cancellation_point();
/* Let nxsem_timedout() do the work */
ret = nxsem_clockwait(sem, clockid, abstime);
if (ret < 0)
{
set_errno(-ret);
ret = ERROR;
}
leave_cancellation_point();
return ret;
}
```
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883739218
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
+ return ret;
Review Comment:
It is not. It returns `OK` (that is `0`) or `ERROR` (that is `-1`) and set errno by `set_errno(-ret);`
So I do not see any POSIX violation here
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883718481
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
Review Comment:
sem_wait is required to set the errno value to the (negated) value returned by nxsem_wait()
(Only in the event of a failure. The errno must not be modified if there is no 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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched: remove nested cancelation point support
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r889366588
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
Done
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883737447
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
+ return ret;
Review Comment:
sem_wait() is required by POSIX to return either 0 (OK) or -1 (ERROR) and to set the errno value appropriately ... It must not return -EINTR or -ECANCELED.
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883737447
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
+ return ret;
Review Comment:
sem_wait() is required by POSIX to return either 0 or -1 and to set the errno value appopriately ... It must not return -EINTR or -ECANCELED.
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883733396
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
For now this is the only place. But I can add a check in others
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883714456
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
Review Comment:
No.. nevermind. I am wrong. In the case of a pending cancellation, enter_cancellation_point() will call exit() and enter_cancellation_point() will never return.
enter_cancellation_point() whould only return if there is no pending cancellation or if this this is nested cancellation (which should not happen).
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883739218
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
+ return ret;
Review Comment:
It is not. It returns `OK` or `ERROR` (that is `-1`) and set errno by `set_errno(-ret);`
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883717580
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
Review Comment:
I wonder if this should be replaced with a DEBUGASSERT? There is a a nesting problem if this does occur.
If nested cancellations are not supported, then there are other changes that should be made. The TCB cpcount could be a boolean since it should be only true or false. The tests for cpcount < INT16_MAX should be removed.
--
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
[GitHub] [incubator-nuttx] patacongo commented on a diff in pull request #6338: sched/semaphore: sem_wait should not report ECANCELED
Posted by GitBox <gi...@apache.org>.
patacongo commented on code in PR #6338:
URL: https://github.com/apache/incubator-nuttx/pull/6338#discussion_r883718481
##########
sched/semaphore/sem_wait.c:
##########
@@ -251,37 +251,21 @@ int nxsem_wait_uninterruptible(FAR sem_t *sem)
int sem_wait(FAR sem_t *sem)
{
- int errcode;
int ret;
/* sem_wait() is a cancellation point */
- if (enter_cancellation_point())
- {
-#ifdef CONFIG_CANCELLATION_POINTS
- /* If there is a pending cancellation, then do not perform
- * the wait. Exit now with ECANCELED.
- */
-
- errcode = ECANCELED;
- goto errout_with_cancelpt;
-#endif
- }
+ enter_cancellation_point();
/* Let nxsem_wait() do the real work */
ret = nxsem_wait(sem);
if (ret < 0)
{
- errcode = -ret;
- goto errout_with_cancelpt;
+ set_errno(-ret);
+ ret = ERROR;
}
leave_cancellation_point();
- return OK;
-
-errout_with_cancelpt:
- set_errno(errcode);
- leave_cancellation_point();
- return ERROR;
Review Comment:
sem_wait is required to set the errno value to the (negated) value returned by nxsem_wait()
--
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