nptl: Document AS-safe functions in cancellation.c.

Message ID 4f3ca170-c8d2-f684-4922-a799413dddda@redhat.com
State Superseded
Headers

Commit Message

Carlos O'Donell Oct. 4, 2019, 8:01 p.m. UTC
  In a recent conversation with Mathieu Desnoyer he pointed out that
because write is AS-safe all the wrappers around write should be 
also AS-safe. We don't spell that out explicitly in the comments
for __pthread_enable_asynccancel and __pthread_disable_asynccancel
so I added them here.

OK for master?

8< --- 8< --- 8<
Document in comments that __pthread_enable_asynccancel and
__pthread_disable_asynccancel must be AS-safe in general with
the exception of the act of cancellation.
---
 ChangeLog           | 5 +++++
 nptl/cancellation.c | 8 ++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)
  

Comments

Zack Weinberg Oct. 4, 2019, 8:45 p.m. UTC | #1
On Fri, Oct 4, 2019 at 4:01 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> In a recent conversation with Mathieu Desnoyer he pointed out that
> because write is AS-safe all the wrappers around write should be
> also AS-safe. We don't spell that out explicitly in the comments
> for __pthread_enable_asynccancel and __pthread_disable_asynccancel
> so I added them here.
>
>
>  /* The next two functions are similar to pthread_setcanceltype() but
>     more specialized for the use in the cancelable functions like write().
> -   They do not need to check parameters etc.  */
> +   They do not need to check parameters etc.  This function must be
> +   AS-safe, with the exception of the actual cancellation, because they
> +   are called by wrappers around AS-safe functions like write().*/

Grammar nit: Either "_These functions_ must be AS-safe, ..."  or
"because _it is_ called."  The former is probably better because
consistent with the first sentence in this comment...

> -
> +/* This function must be AS-safe, with the exception of the actual
> +   cancellation, because they are called by wrappers around AS-safe
> +   functions like write().*/

... but then maybe you _don't_ want to add this comment as well.

zw
  

Patch

diff --git a/ChangeLog b/ChangeLog
index bdd97f0606..716f912c40 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-10-04  Carlos O'Donell  <carlos@redhat.com>
+
+	* nptl/cancellation.c: Document that all functions here must be
+	AS-safe because they wrap AS-safe functions.
+
 2019-10-03  Leandro Pereira  <leandro.pereira@microsoft.com>
 
 	* elf/dl-load.c: Use __pread64_nocancel() instead of __lseek()+
diff --git a/nptl/cancellation.c b/nptl/cancellation.c
index 7712845561..ffe5b78b18 100644
--- a/nptl/cancellation.c
+++ b/nptl/cancellation.c
@@ -24,7 +24,9 @@ 
 
 /* The next two functions are similar to pthread_setcanceltype() but
    more specialized for the use in the cancelable functions like write().
-   They do not need to check parameters etc.  */
+   They do not need to check parameters etc.  This function must be
+   AS-safe, with the exception of the actual cancellation, because they
+   are called by wrappers around AS-safe functions like write().*/
 int
 attribute_hidden
 __pthread_enable_asynccancel (void)
@@ -59,7 +61,9 @@  __pthread_enable_asynccancel (void)
   return oldval;
 }
 
-
+/* This function must be AS-safe, with the exception of the actual
+   cancellation, because they are called by wrappers around AS-safe
+   functions like write().*/
 void
 attribute_hidden
 __pthread_disable_asynccancel (int oldtype)