Re: BZ#15819: introduce internal function to ease poll retry with timeout

[Roland McGrath <roland at hack dot frob dot com>]

> > 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