This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] nptl: Fix join/tryjoin comments.
- From: Carlos O'Donell <carlos at redhat dot com>
- To: GNU C Library <libc-alpha at sourceware dot org>, Florian Weimer <fweimer at redhat dot com>
- Date: Mon, 11 Feb 2019 16:11:27 -0500
- Subject: [PATCH] nptl: Fix join/tryjoin comments.
Florian,
I noticed these comment mistakes while reviewing your patch for BZ #24211.
OK for master?
~~~
This commit fixes several inaccurate comments in the implementation of
__pthread_timedjoin_ex(), pthread_tryjoin_np, and lll_wait_tid, all of
which are used to implement POSIX thread joining.
Firstly, in pthread_tryjoin_np() we only attempt the join if a read of
pd->tid == 0, because that means the thread has already been reaped by
the kernel and we can safely join it without blocking.
Secondly, all join implementations call the common
__pthread_timedjoin_ex and only if abstime is NULL (block) do we
actually need to use cancellation (to cancel the potentially infinite
wait).
Lastly, in lll_wait_tid we only call the canellation futex operations if
abstime is non-NULL.
With the comments corrected the implementation notes are consistent and
logical.
Signed-off-by: Carlos O'Donell <carlos@redhat.com>
---
ChangeLog | 7 +++++++
nptl/lll_timedwait_tid.c | 3 +--
nptl/pthread_join_common.c | 4 ++--
nptl/pthread_tryjoin.c | 2 +-
sysdeps/nptl/lowlevellock.h | 2 +-
5 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 6f1d967f62..37e7babf75 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-11 Carlos O'Donell <carlos@redhat.com>
+
+ * nptl/lll_timedwait_tid.c: Use GNU-style comment.
+ * nptl/pthread_join_common.c: Fix comment.
+ * nptl/pthread_tryjoin.c: Likewise.
+ * sysdeps/nptl/lowlevellock.h: Likewise.
+
2019-02-11 Paul A. Clarke <pc@us.ibm.com>
* sysdeps/powerpc/fpu/e_sqrt.c (__slow_ieee754_sqrtf):
diff --git a/nptl/lll_timedwait_tid.c b/nptl/lll_timedwait_tid.c
index 59ecbb4446..8f22da9b05 100644
--- a/nptl/lll_timedwait_tid.c
+++ b/nptl/lll_timedwait_tid.c
@@ -60,8 +60,7 @@ __lll_timedwait_tid (int *tidp, const struct timespec *abstime)
/* If *tidp == tid, wait until thread terminates or the wait times out.
The kernel up to version 3.16.3 does not use the private futex
- operations for futex wake-up when the clone terminates.
- */
+ operations for futex wake-up when the clone terminates. */
if (lll_futex_timed_wait (tidp, tid, &rt, LLL_SHARED) == -ETIMEDOUT)
return ETIMEDOUT;
}
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index ecb78ffba5..e99854ac39 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -76,8 +76,8 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
if (block)
{
- /* During the wait we change to asynchronous cancellation. If we
- are cancelled the thread we are waiting for must be marked as
+ /* If abstime is NULL we switch to asynchronous cancellation. If we
+ are cancelled then the thread we are waiting for must be marked as
un-wait-ed for again. */
pthread_cleanup_push (cleanup, &pd->joinid);
diff --git a/nptl/pthread_tryjoin.c b/nptl/pthread_tryjoin.c
index aa4fe07af5..7dbf22868d 100644
--- a/nptl/pthread_tryjoin.c
+++ b/nptl/pthread_tryjoin.c
@@ -26,7 +26,7 @@ pthread_tryjoin_np (pthread_t threadid, void **thread_return)
if (pd->tid != 0)
return EBUSY;
- /* If pd->tid != 0 then lll_wait_tid will not block on futex
+ /* If pd->tid == 0 then lll_wait_tid will not block on futex
operation. */
return __pthread_timedjoin_ex (threadid, thread_return, NULL, false);
}
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index 36bbc472e7..2581ed5ca9 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -185,7 +185,7 @@ extern int __lll_timedwait_tid (int *, const struct timespec *)
operations for futex wake-up when the clone terminates.
If ABSTIME is not NULL, is used a timeout for futex call. If the timeout
occurs then return ETIMEOUT, if ABSTIME is invalid, return EINVAL.
- The futex operation are issues with cancellable versions. */
+ If abstime is NULL we use a cancellable futex wait operation. */
#define lll_wait_tid(tid, abstime) \
({ \
int __res = 0; \
--
2.20.1
--
Cheers,
Carlos.