From patchwork Mon Oct 7 05:35:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 34837 Received: (qmail 9302 invoked by alias); 7 Oct 2019 05:35:34 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 9290 invoked by uid 89); 7 Oct 2019 05:35:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=carlosredhatcom, sk:carlos, carlos@redhat.com, U*carlos X-HELO: us-smtp-delivery-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1570426531; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=iL2NhGVWpLH9cehmyJinnp7BKo+mRjLqFNm4Xv2klP0=; b=KeioVaFa5ApZSfTwp/MPerD+uEE00k0XCwDRVZsT6EVbkOM6zDMj2qAFwCaTusAjGGfPeL VcfVFx2Pj5bxV/2HxbLDj6Ip+OwjGWd5v/44OI+gl7h371Jmj/gJry5IF7AG6WlqUjMDAz Tr3YPC9v+wBNdoxxJR1UdJtF7pBMsHM= Return-Path: Subject: Re: [PATCH] nptl: Document AS-safe functions in cancellation.c. To: Zack Weinberg Cc: libc-alpha References: <4f3ca170-c8d2-f684-4922-a799413dddda@redhat.com> From: Carlos O'Donell Message-ID: <5ad2611d-bc28-524d-674e-9fa1f7a9add5@redhat.com> Date: Mon, 7 Oct 2019 01:35:26 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 On 10/4/19 4:45 PM, Zack Weinberg wrote: > On Fri, Oct 4, 2019 at 4:01 PM Carlos O'Donell 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 + + * nptl/cancellation.c: Document that all functions here must be + AS-safe because they wrap AS-safe functions. + 2019-10-07 Leandro Pereira * 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)