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

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Jul 17 22:55, Mark Geisert wrote:
> Corinna Vinschen wrote:
> > On Jul 15 01:20, Mark Geisert wrote:
> > > +  //XXX ... and infinite timeout?
> > 
> > Yes, if timeout is a NULL pointer.
> 
> My XXX concern was whether an app could get stuck here and not be abortable.
> But I take your comments to mean a non-maskable signal will break out of the
> sigtimedwait(), so e.g. Ctrl-C, or SIGTERM from outside, could interrupt the
> app.

At least SIGKILL should work.

> > > +  res = sigtimedwait (&sigmask, &si, to);
> > > +  if (res == -1)
> > > +    return -1; /* Return with errno set by failed sigtimedwait() */
> > > +  time1 = GetTickCount ();
> > 
> > This is unsafe.  As a 32 bit function GetTickCount wraps around roughly
> > every 49 days.  Use ULONGLONG GetTickCount64() instead.
> 
> OK, will fix.
> 
> > > +  /* Adjust timeout to account for time just waited */
> > > +  msecs -= (time1 - time0);
> > > +  if (msecs < 0)
> > 
> > This can't happen then.
> 
> Right.
> 
> > > +  to->tv_sec = msecs / 1000;
> > > +  to->tv_nsec = (msecs % 1000) * 1000000;
> > 
> > Uh oh, you're changing caller values, despite timeout being const.
> > `to' shouldn't be a pointer, but a local struct timespec instead.
> 
> I'll revisit this issue.  This internal aiosuspend() routine is called from
> both aio_suspend() and lio_listio().  Those two functions have conflicting
> protections on args passed to them and I had some trouble coming up with
> something that would compile cleanly.  As I say, I will look at this again.

Local var should work.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature