* [PATCH] nptl: Document AS-safe functions in cancellation.c.
@ 2019-10-04 20:01 Carlos O'Donell
2019-10-04 20:45 ` Zack Weinberg
0 siblings, 1 reply; 5+ messages in thread
From: Carlos O'Donell @ 2019-10-04 20:01 UTC (permalink / raw)
To: libc-alpha
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(-)
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)
--
2.21.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nptl: Document AS-safe functions in cancellation.c.
2019-10-04 20:01 [PATCH] nptl: Document AS-safe functions in cancellation.c Carlos O'Donell
@ 2019-10-04 20:45 ` Zack Weinberg
2019-10-07 5:35 ` Carlos O'Donell
0 siblings, 1 reply; 5+ messages in thread
From: Zack Weinberg @ 2019-10-04 20:45 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nptl: Document AS-safe functions in cancellation.c.
2019-10-04 20:45 ` Zack Weinberg
@ 2019-10-07 5:35 ` Carlos O'Donell
2019-10-07 16:58 ` Zack Weinberg
0 siblings, 1 reply; 5+ messages in thread
From: Carlos O'Donell @ 2019-10-07 5:35 UTC (permalink / raw)
To: Zack Weinberg; +Cc: libc-alpha
On 10/4/19 4:45 PM, Zack Weinberg wrote:
> 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...
Yes, I actually wrote "These" first, but then cut-and-pasted it into
both functions, and rewrote "These" to "This" and forgot to clean it up.
Thanks for the review!
>> -
>> +/* 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.
v2
- Rework comments based on Zack's review.
How about v2?
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 | 7 +++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index cfabdaddf4..2ce48223f4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-10-07 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-07 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..23d7b2b34c 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. These functions 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,8 @@ __pthread_enable_asynccancel (void)
return oldval;
}
-
+/* See the comment for __pthread_enable_asynccancel regarding
+ the AS-safety of this function. */
void
attribute_hidden
__pthread_disable_asynccancel (int oldtype)
--
2.21.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nptl: Document AS-safe functions in cancellation.c.
2019-10-07 5:35 ` Carlos O'Donell
@ 2019-10-07 16:58 ` Zack Weinberg
2019-10-18 20:03 ` Carlos O'Donell
0 siblings, 1 reply; 5+ messages in thread
From: Zack Weinberg @ 2019-10-07 16:58 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: libc-alpha
On Mon, Oct 7, 2019 at 1:35 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> How about v2?
Revised patch LGTM.
zw
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nptl: Document AS-safe functions in cancellation.c.
2019-10-07 16:58 ` Zack Weinberg
@ 2019-10-18 20:03 ` Carlos O'Donell
0 siblings, 0 replies; 5+ messages in thread
From: Carlos O'Donell @ 2019-10-18 20:03 UTC (permalink / raw)
To: Zack Weinberg; +Cc: libc-alpha
On 10/7/19 12:58 PM, Zack Weinberg wrote:
> On Mon, Oct 7, 2019 at 1:35 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> How about v2?
>
> Revised patch LGTM.
Pushed. Thanks.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-18 20:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 20:01 [PATCH] nptl: Document AS-safe functions in cancellation.c Carlos O'Donell
2019-10-04 20:45 ` Zack Weinberg
2019-10-07 5:35 ` Carlos O'Donell
2019-10-07 16:58 ` Zack Weinberg
2019-10-18 20:03 ` Carlos O'Donell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).