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/04/22 15:18:23 UTC
[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request, #6137: sched: remove DEBUGASSERT from nx_waitpid
pkarashchenko opened a new pull request, #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137
## Summary
`stat_loc` of `wait` and `waitpid` can be `NULL` according to POSIX. Remove `DEBUGASSERT(stat_loc);` from `nx_waitpid`
## Impact
Minimal if this was not reported by anyone till now
## Testing
Pass CI
Tested with SAME70-QMTECH board
--
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 pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137#issuecomment-1106715671
> @pkarashchenko Could you please link a reference to the POSIX chapter that states that `stat_loc` may be `NULL`?
I've updated PR description
--
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] gustavonihei commented on pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137#issuecomment-1106728981
> > @pkarashchenko Could you please link a reference to the POSIX chapter that states that `stat_loc` may be `NULL`?
>
> I've updated PR description
Thanks, I had skimmed that page, but I failed to find the information.
The excerpt you pasted does not state that `stat_loc` may be `NULL`. It just determines the behavior when `stat_loc` is non-NULL.
I tried looking at other implementations. In the Linux Kernel, if `stat_loc` (`stat_addr` in this case) is `NULL`, the kernel returns `-EFAULT` to the userspace:
https://github.com/torvalds/linux/blob/master/kernel/exit.c#L1683
--
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 pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137#issuecomment-1106743592
@gustavonihei I think if POSIX want to define `start_loc` to be non-NULL, then the error code section would state that explicitly. And there would be no sentence that I pasted in the PR description.
--
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] gustavonihei commented on pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137#issuecomment-1106709417
@pkarashchenko Could you please link a reference to the aforementioned POSIX chapter that states that `stat_loc` may be `NULL`?
--
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] gustavonihei merged pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
gustavonihei merged PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137
--
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] gustavonihei commented on pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137#issuecomment-1106743590
> Yeah, but POSIX does not define an error code if `start_loc` is `NULL`, so Linux interpreted that to an error code. For me it is natural that if `start_loc` is `NULL` then the caller "does not care" about exit status.
I agree with your point.
In summary:
- Prior to this PR, `wait` and `waitpid` had a premise of `stat_loc` being non-NULL. Passing a NULL pointer resulted in undefined behavior when DEBUG assertions are not enabled.
- This PR defines a proper behavior for a NULL `stat_loc`.
I am okay with the proposed changes.
--
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 pull request #6137: sched: remove DEBUGASSERT from nx_waitpid
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6137:
URL: https://github.com/apache/incubator-nuttx/pull/6137#issuecomment-1106732583
Yeah, but POSIX does not define an error code if `start_loc` is `NULL`, so Linux interpreted that to an error code.
For me it is natural that if `start_loc` is `NULL` then the caller "does not care" about exit status.
--
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