[PATCH v3 1/3] POSIX Asynchronous I/O support: aio files

[Mark Geisert]

Corinna Vinschen wrote:
> Mark,
>
> I think there's a bug in sigtimedwait.  I just found the problem while
> looking into this aio_suspend stuff:
>
> On Jul 17 16:51, Corinna Vinschen wrote:
>>> +  res = sigtimedwait (&sigmask, &si, to);
>
> You're giving the timeout value verbatim to sigtimedwait().
>
> Let's have a look into sigtimedwait, per your original patch, commit
> 24ff42d79aab:
>
> +  if (timeout)
> +    {
> +      if (timeout->tv_sec < 0
> +           || timeout->tv_nsec < 0 || timeout->tv_nsec > (NSPERSEC * 100LL))
> +       {
> +         set_errno (EINVAL);
> +         return -1;
> +       }
>
> So we're enforcing a positive timeout value.
>
> +      /* convert timespec to 100ns units */
> +      waittime.QuadPart = (LONGLONG) timeout->tv_sec * NSPERSEC
> +                          + ((LONGLONG) timeout->tv_nsec + 99LL) / 100LL;
> +    }
>
> ...which is converted to a likewise positive 100ns interval ...
>
> +  return sigwait_common (set, info, timeout ? &waittime : cw_infinite);
>
> ...given to sigwait_common, which in turn calls
>
>   cygwait (NULL, waittime, [flags])
>
> cygwait uses this waittime value verbatim in a call to
>
>   NtSetTimer (_my_tls.locals.cw_timer, timeout, [...]);
>
> So NtSetTimer is called with a *positive* waittime value, right?
>
> A positive value given to NtSetTimer is evaluated as a timestamp,
> *not* as a time interval.  Look at the lpDueTime description of
> SetWaitableTimerEx.  That's the WIN32 API function exposing the
> NtSetTimer function:
> https://docs.microsoft.com/en-us/windows/desktop/api/synchapi/nf-synchapi-setwaitabletimerex
>
> However, the POSIX description of sigtimedwait indicates that the
> timespec value is evaluated as a time interval, which is a relative
> time:
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/sigtimedwait.html
>
> So bottom line is, shouldn't timeout be converted to a negative waittime
> value in sigtimedwait?

Yes, you are correct.  I did not dig deeply enough into cygwait to notice my error.

Is it OK for me to fix this as part of the AIO patch set or should it be 
separate?  Either way is fine with me.
Thanks again,

..mark