From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13488 invoked by alias); 18 Nov 2016 14:21:14 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 13467 invoked by uid 89); 18 Nov 2016 14:21:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=cancel X-HELO: mx1.redhat.com Message-ID: <1479478867.7146.1167.camel@localhost.localdomain> Subject: Re: [PATCH v7] getrandom system call wrapper [BZ #17252] From: Torvald Riegel To: Szabolcs Nagy Cc: Adhemerval Zanella , nd@arm.com, libc-alpha@sourceware.org Date: Fri, 18 Nov 2016 14:21:00 -0000 In-Reply-To: <582EBBB5.80309@arm.com> References: <3dd5d3b8-c196-98fb-1671-90cd90ab90c7@redhat.com> <244f578c-889a-a4cf-c686-bb2a5e49cca1@panix.com> <2d175242-1a82-9410-d01e-682ab4d9081e@panix.com> <1479319342.7146.1143.camel@localhost.localdomain> <950ffaeb-8984-7840-e495-12442f821c9c@linaro.org> <1479387147.7146.1147.camel@localhost.localdomain> <582EBBB5.80309@arm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-11/txt/msg00655.txt.bz2 On Fri, 2016-11-18 at 08:28 +0000, Szabolcs Nagy wrote: > On 17/11/16 12:52, Torvald Riegel wrote: > > On Wed, 2016-11-16 at 17:52 -0200, Adhemerval Zanella wrote: > >> > >> On 16/11/2016 16:02, Torvald Riegel wrote: > >>> On Wed, 2016-11-16 at 16:52 +0100, Florian Weimer wrote: > >>>> On 11/16/2016 04:20 PM, Zack Weinberg wrote: > >>>>> On 11/16/2016 10:11 AM, Florian Weimer wrote: > >>>>>> On 11/14/2016 07:29 PM, Zack Weinberg wrote: > >>>>>>> On 11/14/2016 12:44 PM, Florian Weimer wrote: > >>>>>>>> This patch switches back to the ssize_t return time. This goes against > >>>>>>>> Theodore Ts'o preference, but seems to reflect the consensus from the > >>>>>>>> largery community. > >>>>>>> > >>>>>>> I still don't think this function should be a cancellation point. > >>>>>> > >>>>>> I guess we'll have to agree to disagree on this matter. > >>>>> > >>>>> I am seriously considering escalating my disagreement here to a formal > >>>>> objection. I would like to know why you think it is NECESSARY - not > >>>>> merely convenient or consistent with other stuff - for this function to > >>>>> be a cancellation point. > >>>> > >>>> It's necessary if you ever want to cancel a hanging getrandom in a > >>>> context where you cannot install a signal handler (so that you can > >>>> trigger EINTR when getrandom is stuck). > >>> > >>> I think it would be better to split the getrandom that is a cancellation > >>> point into two functions, getrandom (not a cancellation point) and > >>> getrandom_cancelable (is a cancellation point). This way, the > >>> functionality is available for programs but requires explicit opt-in, > >>> while the default is not going to lead to surprises in the expected > >>> common case (ie, when randomness is available). I don't think the > >>> opt-in is a problem because as you said, cancellation requires careful > >>> programming anyway. > >>> > >> > >> I would advise against adding another symbol, it only adds complexity > >> and most likely one interface would be preferable from application > >> point of view. > > > > We could add a parameter too that determines whether it is a > > cancellation point. That avoids your concern about adding another > > why? > > use pthread_setcancelstate. > > works with all cancellation points not just getrandom. As discussed in the thread, there are different opinions about what the default should be. There are reasonable arguments for both options. In such a case, it seems better to make the choice explicit, simply from an ease-of-use and interface design perspective. This is not about whether it's possible for users to do (they could build their own syscall wrappers after all too, right? ;) ) but about clean interfaces. With your proposal, one could argue that, for example, every library has to be aware of cancellation and how it works if one of the clients of the library could want to use cancellation; the library either has to disable it explicitly, or it has to document which additional cancellation points exist and has to implement the cleanup handlers. This is error-prone and adds complexity to those use cases. Therefore, it seems better to avoid that potential source of bugs and either make the default to not support cancellation (ie, an opt-in for the cancellation facility), or make at least make the choice explicit for users of getrandom (ie, two functions or an additional parameter).