Incorrect behavior in TIOCINQ ioctl

Incorrect behavior in TIOCINQ ioctl

[Åke Rehnman]

Hi,

I recently ran in to some troubles with the TIOCINQ ioctl. I am 
wondering if the cygwin implementation is correct... It seems if there 
were any existing framing overrun errors etc etc  before calling the 
TIOCINQ ioctl it is returning an error (EINVAL). Reading through linux 
implmentation of TIOCINQ does simply return number of pending chars 
without any clearing or checking for errors.

I suggest the whole if (ev & CE_FRAME  ...... ) is removed.

Excerpt from fhandler_serial.cc:

/* ioctl: */
int
fhandler_serial::ioctl (unsigned int cmd, void *buf)
{
.
.
   if (!ClearCommError (get_handle (), &ev, &st))
     {
       __seterrno ();
       res = -1;
     }
.
.
.
      case TIOCINQ:
        if (ev & CE_FRAME || ev & CE_IOE || ev & CE_OVERRUN || ev & 
CE_RXOVER
        || ev & CE_RXPARITY)
      {
        set_errno (EINVAL);    /* FIXME: Use correct errno */
        res = -1;
      }
      else
         ipbuf = st.cbInQue;
      break;



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Incorrect behavior in TIOCINQ ioctl

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]

On Feb 27 22:38, Åke Rehnman wrote:
> Hi,
> 
> I recently ran in to some troubles with the TIOCINQ ioctl. I am wondering if
> the cygwin implementation is correct... It seems if there were any existing
> framing overrun errors etc etc  before calling the TIOCINQ ioctl it is
> returning an error (EINVAL). Reading through linux implmentation of TIOCINQ
> does simply return number of pending chars without any clearing or checking
> for errors.
> 
> I suggest the whole if (ev & CE_FRAME  ...... ) is removed.
> 
> Excerpt from fhandler_serial.cc:
> 
> /* ioctl: */
> int
> fhandler_serial::ioctl (unsigned int cmd, void *buf)
> {
> .
> .
>   if (!ClearCommError (get_handle (), &ev, &st))
>     {
>       __seterrno ();
>       res = -1;
>     }
> .
> .
> .
>      case TIOCINQ:
>        if (ev & CE_FRAME || ev & CE_IOE || ev & CE_OVERRUN || ev & CE_RXOVER
>        || ev & CE_RXPARITY)
>      {
>        set_errno (EINVAL);    /* FIXME: Use correct errno */
>        res = -1;
>      }
>      else
>         ipbuf = st.cbInQue;
>      break;

I'm not familiar with serial I/O and the code is pretty stable(*).

- Is it a safe bet that ClearCommError returns valid values in
  st.cbInQue even if one of the error conditions occur?  Maybe the
  right thing to do is to return 0 in certain error cases...?

- Did you actually try if this fixes your problem?  It's pretty
  simple to build the Cygwin DLL
  https://cygwin.com/faq.html#faq.programming.building-cygwin


Corinna

(*) euphemistically for "nobody looked into the code for a long time"


-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Incorrect behavior in TIOCINQ ioctl

[Åke Rehnman]

On 2020-02-28 20:23, Corinna Vinschen wrote:

> - Is it a safe bet that ClearCommError returns valid values in
>    st.cbInQue even if one of the error conditions occur?  Maybe the
>    right thing to do is to return 0 in certain error cases...?

The win32 api  documentation does not mention anything about not 
returning proper device status as long as the function succeeds...

Anyhow in my pursuit for the truth I whipped up a small test program and 
strace:d it (with some extra debug printout in fhandler_serial::ioctl)

  127 2403456 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=7
   216 3406481 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=E
    90 4409676 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=15
   141 5413027 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=1C
   129 6416204 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=23
   121 7419254 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=2A
   203 8423829 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=31
    89 9427183 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=38
   131 10431271 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=3F
   118 11435254 [main] python2.7 1662 fhandler_serial::ioctl: 0 = 
ioctl(541B, 0xFFFFBCB0) ev=C st.cbInQue=46

>
> - Did you actually try if this fixes your problem?  It's pretty
>    simple to build the Cygwin DLL
>    https://cygwin.com/faq.html#faq.programming.building-cygwin

I patched it yesterday and it solved the problem...

/Ake


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Incorrect behavior in TIOCINQ ioctl

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]

On Feb 29 11:37, Åke Rehnman wrote:
> 
> On 2020-02-28 20:23, Corinna Vinschen wrote:
> 
> > - Is it a safe bet that ClearCommError returns valid values in
> >    st.cbInQue even if one of the error conditions occur?  Maybe the
> >    right thing to do is to return 0 in certain error cases...?
> 
> The win32 api  documentation does not mention anything about not returning
> proper device status as long as the function succeeds...
> 
> Anyhow in my pursuit for the truth I whipped up a small test program and
> strace:d it (with some extra debug printout in fhandler_serial::ioctl)
> 
>  127 2403456 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=7
>   216 3406481 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=E
>    90 4409676 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=15
>   141 5413027 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=1C
>   129 6416204 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=23
>   121 7419254 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=2A
>   203 8423829 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=31
>    89 9427183 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=38
>   131 10431271 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=3F
>   118 11435254 [main] python2.7 1662 fhandler_serial::ioctl: 0 = ioctl(541B,
> 0xFFFFBCB0) ev=C st.cbInQue=46
> 
> > 
> > - Did you actually try if this fixes your problem?  It's pretty
> >    simple to build the Cygwin DLL
> >    https://cygwin.com/faq.html#faq.programming.building-cygwin
> 
> I patched it yesterday and it solved the problem...
> 
> /Ake

I pushed a patch.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]