Incorrect behavior in TIOCINQ ioctl

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Reply | Threaded
Open this post in threaded view
|

Re: Incorrect behavior in TIOCINQ ioctl

Corinna Vinschen-2
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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Reply | Threaded
Open this post in threaded view
|

Re: Incorrect behavior in TIOCINQ ioctl

Corinna Vinschen-2
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

signature.asc (849 bytes) Download Attachment