Setting termios VMIN > 0 and VTIME > 0 on non blocking file

[Åke Rehnman]

On 2020-03-13 11:12, Corinna Vinschen wrote:
> On Mar 12 18:04, Åke Rehnman via Cygwin wrote:
>> On 2020-03-12 16:08, Corinna Vinschen wrote:
>>> On Mar 12 15:44, Corinna Vinschen wrote:
>>>> On Mar 12 15:20, Åke Rehnman via Cygwin wrote:
>>>>> I think the problem is if the number of bytes requested are more than what
>>>> To clarify: number of bytes == VMIN?
>> no number of bytes requested from ReadFile(). As far as I know Win32 has no
>> concept of VMIN.
> Right, just this weird TimeoutMultiplier, but nevertheless I wasn't sure
> what you meant.
>
>>>>> is in the buffer it is going to overlap the read function (because of VTIME)
>>>>> and immediately after that CancelIO is called. Contrary to what is mentioned
>>>>> in the source code I think CancelIO is actually discarding data...
>>>> So far we didn't have that experience.  CancelIO is usually safe
>>>> in this regard.
>> The data is MIA somehow...
> Yes, but we're calling CancelIo in other circumstances in Cygwin and
> there were no reports of missing data.  CancelIo is just supposed to
> terminate the currently running overlapped IO, not to discard any
> in-flight data.  If that's different for serial IO, there would be no
> way to terminate serial overlapped IO gracefully.  Well, yeah, it's
> Windows, but still...
I have scrutinized the microsoft serial driver reference implementation 
(https://github.com/microsoft/Windows-driver-samples/blob/master/serial/serial/read.c) 
and it looks to me as CancelIo does not purge any data. However since 
the driver is vendor dependent in my case FTDI it is impossible to know 
exactly what is going on since it is closed source.
>>>>> -    if ((vmin_ > 0) && (vtime_ == 0))
>>>>> +       if (is_nonblocking())
>>>>> +       {
>>>>> +               to.ReadIntervalTimeout = MAXDWORD;
>>>>> +       }
>>>>> +    else if ((vmin_ > 0) && (vtime_ == 0))
>>>> What if you switch to !O_NONBLOCK after calling tcsetattr?  The
>>>> setting of ReadIntervalTimeout would be lost then.
>>>>
>>>> Either we have to repeat calling SetCommTimeouts every time
>>>> we switch mode, or we have to do the above setting temporary
>>>> every time we call ReadFile in non blocking mode.
>> True.
>>> What about this:
>>>
>>> diff --git a/winsup/cygwin/fhandler_serial.cc b/winsup/cygwin/fhandler_serial.cc
>>> --- a/winsup/cygwin/fhandler_serial.cc
>>> +++ b/winsup/cygwin/fhandler_serial.cc
>>> @@ -68,6 +68,16 @@ fhandler_serial::raw_read (void *ptr, size_t& ulen)
>>>    	goto err;
>>>          else if (ev)
>>>    	termios_printf ("error detected %x", ev);
>>> +      else if (is_nonblocking ())
>>> +	{
>>> +	  if (!st.cbInQue)
>>> +	    {
>>> +	      tot = -1;
>>> +	      set_errno (EAGAIN);
>>> +	      goto out;
>>> +	    }
>>> +	  inq = st.cbInQue;
>>> +	}
>>>          else if (st.cbInQue && !vtime_)
>>>    	inq = st.cbInQue;
>>>          else if (!is_nonblocking () && !overlapped_armed)
>> Looks promising. I will try it.
Your patch works (for my test case and screen). Question is if we have 
to consider the case where ulen==0 ...
>>
>> BTW there is a gremlin in the "else if (ev)" line....
> A gremlin?  Would you mind to explain?  Btw., if you find a bug
> in the code, we do take patches :) https://cygwin.com/contrib.html
If we have an error event in ev it will make a blocking read even if 
VTIME==0.