You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "anjiahao1 (via GitHub)" <gi...@apache.org> on 2023/03/10 04:33:05 UTC
[GitHub] [nuttx] anjiahao1 opened a new pull request, #8786: sig_timewait:changes the macro for waitticks
anjiahao1 opened a new pull request, #8786:
URL: https://github.com/apache/nuttx/pull/8786
use SYSTEM_TIME64 inside of LONG LONG better
Sometimes longlong and system_time64 are not uniform
## Summary
changes the macro for waitticks
## Impact
signal
## Testing
sim nsh
--
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] [nuttx] xiaoxiang781216 commented on a diff in pull request #8786: sig_timewait:changes the macro for waitticks
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8786:
URL: https://github.com/apache/nuttx/pull/8786#discussion_r1181463703
##########
sched/signal/sig_timedwait.c:
##########
@@ -320,13 +324,11 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info,
* time in nanoseconds.
*/
-#ifdef CONFIG_HAVE_LONG_LONG
- uint64_t waitticks64 = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
- (uint64_t)timeout->tv_nsec +
- NSEC_PER_TICK - 1) /
- NSEC_PER_TICK;
- DEBUGASSERT(waitticks64 <= UINT32_MAX);
- waitticks = (uint32_t)waitticks64;
+#ifdef CONFIG_SYSTEM_TIME64
+ waitticks = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
+ (uint64_t)timeout->tv_nsec + NSEC_PER_TICK - 1) /
+ NSEC_PER_TICK;
+ DEBUGASSERT(waitticks <= UINT32_MAX);
Review Comment:
@anjiahao1 please fix 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
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #8786: sig_timewait:changes the macro for waitticks
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8786:
URL: https://github.com/apache/nuttx/pull/8786#discussion_r1132089181
##########
sched/signal/sig_timedwait.c:
##########
@@ -319,7 +323,7 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info,
* time in nanoseconds.
*/
-#ifdef CONFIG_HAVE_LONG_LONG
+#ifdef CONFIG_SYSTEM_TIME64
uint64_t waitticks64 = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
(uint64_t)timeout->tv_nsec +
NSEC_PER_TICK - 1) /
Review Comment:
Most probably `waitticks64` now is not needed.
--
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] [nuttx] xiaoxiang781216 merged pull request #8786: sig_timewait:changes the macro for waitticks
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8786:
URL: https://github.com/apache/nuttx/pull/8786
--
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] [nuttx] anjiahao1 commented on a diff in pull request #8786: sig_timewait:changes the macro for waitticks
Posted by "anjiahao1 (via GitHub)" <gi...@apache.org>.
anjiahao1 commented on code in PR #8786:
URL: https://github.com/apache/nuttx/pull/8786#discussion_r1133060871
##########
sched/signal/sig_timedwait.c:
##########
@@ -319,7 +323,7 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info,
* time in nanoseconds.
*/
-#ifdef CONFIG_HAVE_LONG_LONG
+#ifdef CONFIG_SYSTEM_TIME64
uint64_t waitticks64 = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
(uint64_t)timeout->tv_nsec +
NSEC_PER_TICK - 1) /
Review Comment:
Yes, but I think it's better to use `system_time64` here than `have long long`
--
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] [nuttx] pkarashchenko commented on a diff in pull request #8786: sig_timewait:changes the macro for waitticks
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8786:
URL: https://github.com/apache/nuttx/pull/8786#discussion_r1133071780
##########
sched/signal/sig_timedwait.c:
##########
@@ -319,7 +323,7 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info,
* time in nanoseconds.
*/
-#ifdef CONFIG_HAVE_LONG_LONG
+#ifdef CONFIG_SYSTEM_TIME64
uint64_t waitticks64 = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
(uint64_t)timeout->tv_nsec +
NSEC_PER_TICK - 1) /
Review Comment:
I mean that waitticks is now uint64_t, so temporary variable can 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] [nuttx] pkarashchenko commented on a diff in pull request #8786: sig_timewait:changes the macro for waitticks
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8786:
URL: https://github.com/apache/nuttx/pull/8786#discussion_r1181172952
##########
sched/signal/sig_timedwait.c:
##########
@@ -242,7 +242,11 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info,
sigset_t intersection;
FAR sigpendq_t *sigpend;
irqstate_t flags;
+#ifdef CONFIG_SYSTEM_TIME64
+ int64_t waitticks;
+#else
int32_t waitticks;
+#endif
Review Comment:
Just as an improvement `sclock_t waitticks;` could be used
##########
sched/signal/sig_timedwait.c:
##########
@@ -320,13 +324,11 @@ int nxsig_timedwait(FAR const sigset_t *set, FAR struct siginfo *info,
* time in nanoseconds.
*/
-#ifdef CONFIG_HAVE_LONG_LONG
- uint64_t waitticks64 = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
- (uint64_t)timeout->tv_nsec +
- NSEC_PER_TICK - 1) /
- NSEC_PER_TICK;
- DEBUGASSERT(waitticks64 <= UINT32_MAX);
- waitticks = (uint32_t)waitticks64;
+#ifdef CONFIG_SYSTEM_TIME64
+ waitticks = ((uint64_t)timeout->tv_sec * NSEC_PER_SEC +
+ (uint64_t)timeout->tv_nsec + NSEC_PER_TICK - 1) /
+ NSEC_PER_TICK;
+ DEBUGASSERT(waitticks <= UINT32_MAX);
Review Comment:
Why we still have 32bit limitation in case of 64bit time?
--
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