From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <libc-alpha-return-55175-listarch-libc-alpha=sources.redhat.com@sourceware.org>
Received: (qmail 31337 invoked by alias); 9 Dec 2014 19:31:35 -0000
Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <libc-alpha.sourceware.org>
List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/libc-alpha/>
List-Post: <mailto:libc-alpha@sourceware.org>
List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: libc-alpha-owner@sourceware.org
Received: (qmail 31318 invoked by uid 89); 9 Dec 2014 19:31:34 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2
X-HELO: topped-with-meat.com
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
From: Roland McGrath <roland@hack.frob.com>
To: Alexandre Oliva <aoliva@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: BZ#15819: introduce internal function to ease poll retry with timeout
In-Reply-To: Alexandre Oliva's message of  Friday, 14 November 2014 00:17:52 -0200 <or4mu2zfsf.fsf@free.home>
References: <orioiiahsf.fsf@free.home>
	<20141113221831.D15A62C3B18@topped-with-meat.com>
	<or4mu2zfsf.fsf@free.home>
Message-Id: <20141209193130.4B4792C3A99@topped-with-meat.com>
Date: Tue, 09 Dec 2014 19:31:00 -0000
X-CMAE-Score: 0
X-CMAE-Analysis: v=2.1 cv=SvUDtp+0 c=1 sm=1 tr=0
		a=WkljmVdYkabdwxfqvArNOQ==:117 a=14OXPxybAAAA:8 a=kj9zAlcOel0A:10
		a=hOe2yjtxAAAA:8 a=HtZH0HBE5qEXNGmVnGEA:9 a=CjuIK1q_8ugA:10
X-SW-Source: 2014-12/txt/msg00242.txt.bz2

> > The __ treatment is never necessary for local-scope names in internal
> > source files (including headers).  It only matters for global names, or
> > uses in installed headers.
> 
> I figured names outside the reserved namespace could be used in our
> tests, and then they'd interfere.  I guess the reasoning is that, since
> we control the tests too, we can just fix any such conflicts in the
> tests, rigth?

How would they interfere?  We're not talking about external linkage names.

> This revised patch fixes both of these issues, and it poisons __poll and
> poll at the end of include/poll-noeintr.h.

We don't have any precedent for using '#pragma poison' in libc.  I'm not
particularly against it, but it is something we have not seen before.  It
needs comments explaining what it means and why it's there.  I think we
should also have a consensus policy on use of this feature, which would be
written up on the wiki.

> --- /dev/null
> +++ b/include/poll-noeintr.h
[...]
> +#ifndef _POLL_EINTR_H
> +#define _POLL_EINTR_H

Make the guard macro match the file name.

> +	  /* At least CLOCK_REALTIME should always be supported, but
> +	     if clock_gettime fails for any other reason, the best we
> +	     can do is to retry with a slightly lower timeout, until
> +	     we complete without interruption.  */
> +	  timeout--;
> +	  goto retry_poll;

This could let TIMEOUT go negative, which has the wrong semantics.
You need to cap it at zero.

> +      tsend.tv_sec = tscur.tv_sec + timeout / 1000;
> +      tsend.tv_nsec = tscur.tv_nsec + timeout % 1000 * 1000000L + 500000;

We really should have some inlines/macros for these canonical time
conversion calculations, rather than repeating them.


Thanks,
Roland