serial 04/04: fix: WaitCommEvent() does not work asynchronously; make WaitCommEvent() better usable when TIOCGICOUNT is not implemented
Wolfgang Walter
wine at stwm.de
Fri Dec 12 11:59:31 CST 2008
From a23874779874372faa24d504c2a104a971cc4add Mon Sep 17 00:00:00 2001
From: Wolfgang Walter <wine at stwm.de>
Date: Fri, 12 Dec 2008 13:21:05 +0100
Subject: serial: fix: WaitCommEvent() does not work asynchronously; make
WaitCommEvent() better usable when TIOCGICOUNT is not implemented
1) If WaitCommEvent() is called with LPOVERLAPPED structure it returns if no
event is pending. When the application then receives a the completion event it
may use GetOverlappedResult() to get the status. This status is always
STATUS_PENDING w ith wine even if it should be STATUS_SUCCESS.
To fix it:
wait_on() takes the PIO_STATUS_BLOCK as fifth argument. struct async_commio
has an additional member piosb.
If there are no events pending wait_on() sets the status of the
PIO_STATUS_BLOCK to STATUS_PENDING , then starts a thread with
RtlQueueWorkItem(wait_for_event, commio) and return with STATUS_PENDING to
io_control()
io_control() must not set the status of the PIO_STATUS_BLOCK when wait_on
return s with STATUS_PENDING as the event check thread may change it in
between. In all other cases io_control() sets the status of the
PIO_STATUS_BLOCK as before.
wait_for_event sets the status of the PIO_STATUS_BLOCK when either a event or
an error occurs or when the application calls SetCommMask(handle,0);
2) wait_for_event() should detect when serial fd is not valid any more
The application calls SetCommMask(handle,0) and then closes handle. In this
case a wait_for_event() thread may be still active but its file handle is not
valid any more.
Probably the application should wait for the asynchronous WaitCommEvent to
finis h before closing the handle. But I did not found any clear hint in MSD -
and the application works fine under windows.
First I check the event mask when the thread wakes up from NtDelayExecution().
This alone should probably avoid the problem. But for correctness I try to
detect when get_irq_info, get_modem_status or check_events fail because the
filehandle has gone.
I modified check_events so that it now returns a NTSTATUS instead the found
even ts. They are returned via an OUT argument *ret.
I don't know if there may be still a small window for a race:
app calls SetCommMask(handle,0) app closes handle app opens another
file/device, unix fd is used again
3) make events work better with serial devices not supporting TIOCGICOUNT
Most drivers of usb2serial adapters do not support TIOCGICOUNT nor do pseudo
ter minals.
I modified get_irq_info() so that it behaves in this case as if TIOCGICOUNT is
n ot defined at compilation time: it returns with STATUS_NOT_IMPLEMENTED;
I further modified wait_on() so that if get_irq_info() returns with
STATUS_NOT_I MPLEMENTED wait_on() does not fail with an error if at least
EV_RXCHAR or EV_TXE MPTY is in the waitmask. Or in other words: an applikation
may use EV_BREAK or E V_ERR together with EV_RXCHAR or EV_TXEMPTY.
I found that in our case all vendor dlls always set EV_BREAK|EV_ERR when they
wa it on EV_RXCHAR or EV_TXEMPTY. They never expected SetCommMask(handle,
EV_BREAK| EV_ERR|EV_RXCHAR) to fail. Instead there was a busy loop resetting
the comm-port , calling SetCommMask(), ...
wait_on() still fails when neiter EV_RXCHAR or EV_TXEMPTY is set an
get_irq_info returns STATUS_NOT_IMPLEMENTED because we would wait for ever
otherwise.
---
dlls/ntdll/serial.c | 160 ++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 120 insertions(+), 40 deletions(-)
diff --git a/dlls/ntdll/serial.c b/dlls/ntdll/serial.c
index 7cc8d00..575fa69 100644
--- a/dlls/ntdll/serial.c
+++ b/dlls/ntdll/serial.c
@@ -846,6 +846,7 @@ typedef struct async_commio
DWORD* events;
HANDLE hEvent;
DWORD evtmask;
+ PIO_STATUS_BLOCK piosb;
DWORD mstat;
serial_irq_info irq_info;
} async_commio;
@@ -870,21 +871,27 @@ static NTSTATUS get_irq_info(int fd, serial_irq_info *irq_info)
return STATUS_SUCCESS;
}
TRACE("TIOCGICOUNT err %s\n", strerror(errno));
- status = FILE_GetNtStatus();
+ /* EINVAL means: TIOCGICOUNT is not supported by serial driver */
+ if (errno != EINVAL)
+ status = FILE_GetNtStatus();
#endif
memset(irq_info,0, sizeof(serial_irq_info));
return status;
}
-static DWORD check_events(int fd, DWORD mask,
- const serial_irq_info *new,
- const serial_irq_info *old,
- DWORD new_mstat, DWORD old_mstat)
+static NTSTATUS check_events(int fd, DWORD mask,
+ const serial_irq_info *new,
+ const serial_irq_info *old,
+ DWORD new_mstat, DWORD old_mstat,
+ DWORD *ret, DWORD *notimplemented)
{
- DWORD ret = 0, queue;
+ DWORD queue;
+ NTSTATUS status = STATUS_SUCCESS;
+
+ *ret = 0;
- TRACE("mask 0x%08x\n", mask);
+ TRACE("mask=0x%08x notimplemented=0x%08x\n", mask, *notimplemented);
TRACE("old->rx 0x%08x vs. new->rx 0x%08x\n", old->rx, new->rx);
TRACE("old->tx 0x%08x vs. new->tx 0x%08x\n", old->tx, new->tx);
TRACE("old->frame 0x%08x vs. new->frame 0x%08x\n", old->frame, new->frame);
@@ -893,41 +900,72 @@ static DWORD check_events(int fd, DWORD mask,
TRACE("old->brk 0x%08x vs. new->brk 0x%08x\n", old->brk, new->brk);
TRACE("old->buf_overrun 0x%08x vs. new->buf_overrun 0x%08x\n", old->buf_overrun, new->buf_overrun);
- if (old->brk != new->brk) ret |= EV_BREAK;
- if ((old_mstat & MS_CTS_ON ) != (new_mstat & MS_CTS_ON )) ret |= EV_CTS;
- if ((old_mstat & MS_DSR_ON ) != (new_mstat & MS_DSR_ON )) ret |= EV_DSR;
- if ((old_mstat & MS_RING_ON) != (new_mstat & MS_RING_ON)) ret |= EV_RING;
- if ((old_mstat & MS_RLSD_ON) != (new_mstat & MS_RLSD_ON)) ret |= EV_RLSD;
- if (old->frame != new->frame || old->overrun != new->overrun || old->parity != new->parity) ret |= EV_ERR;
+ if (old->brk != new->brk) *ret |= EV_BREAK;
+ if ((old_mstat & MS_CTS_ON ) != (new_mstat & MS_CTS_ON )) *ret |= EV_CTS;
+ if ((old_mstat & MS_DSR_ON ) != (new_mstat & MS_DSR_ON )) *ret |= EV_DSR;
+ if ((old_mstat & MS_RING_ON) != (new_mstat & MS_RING_ON)) *ret |= EV_RING;
+ if ((old_mstat & MS_RLSD_ON) != (new_mstat & MS_RLSD_ON)) *ret |= EV_RLSD;
+ if (old->frame != new->frame || old->overrun != new->overrun || old->parity != new->parity) *ret |= EV_ERR;
if (mask & EV_RXCHAR)
{
queue = 0;
#ifdef TIOCINQ
if (ioctl(fd, TIOCINQ, &queue))
- WARN("TIOCINQ returned error\n");
+ {
+ if (errno == EINVAL)
+ *notimplemented |= EV_RXCHAR;
+ else
+ {
+ status = FILE_GetNtStatus();
+ WARN("TIOCINQ returned error\n");
+ return status;
+ }
+ }
#endif
if (queue)
- ret |= EV_RXCHAR;
+ *ret |= EV_RXCHAR;
+ TRACE("INQUEUE: %d: %s\n", queue, queue ? "maybe received something" : "received nothing" );
}
if (mask & EV_TXEMPTY)
{
+ int err = EINVAL;
queue = 0;
/* We really want to know when all characters have gone out of the transmitter */
+/* TIOCSERGETLSR is not supported by many drivers, so ignore EINVAL and fallback
+ * to TIOCOUTQ
+ */
#if defined(TIOCSERGETLSR)
- if (ioctl(fd, TIOCSERGETLSR, &queue))
- WARN("TIOCSERGETLSR returned error\n");
- if (queue)
+ if (!ioctl(fd, TIOCSERGETLSR, &queue))
+ {
+ queue = !queue;
+ err = 0;
+ }
+ else
+ {
+ queue = 0;
+ err = errno;
+ }
+#endif
/* TIOCOUTQ only checks for an empty buffer */
-#elif defined(TIOCOUTQ)
- if (ioctl(fd, TIOCOUTQ, &queue))
- WARN("TIOCOUTQ returned error\n");
- if (!queue)
+#if defined(TIOCOUTQ)
+ if (err == EINVAL && ioctl(fd, TIOCOUTQ, &queue))
+ err = errno;
#endif
- ret |= EV_TXEMPTY;
+ if (err == EINVAL)
+ *notimplemented |= EV_TXEMPTY;
+ else if (err != 0)
+ {
+ status = FILE_GetNtStatus();
+ WARN("TIOCOUTQ returned error\n");
+ return status;
+ }
+ if (!queue)
+ *ret |= EV_TXEMPTY;
TRACE("OUTQUEUE %d, Transmitter %sempty\n",
- queue, (ret & EV_TXEMPTY) ? "" : "not ");
+ queue, queue ? "" : "not ");
}
- return ret & mask;
+ *ret &= mask;
+ return status;
}
/***********************************************************************
@@ -941,8 +979,10 @@ static DWORD CALLBACK wait_for_event(LPVOID arg)
{
async_commio *commio = (async_commio*) arg;
int fd, needs_close;
+ NTSTATUS status;
+ DWORD notimplemented;
- if (!server_get_unix_fd( commio->hDevice, FILE_READ_DATA | FILE_WRITE_DATA, &fd, &needs_close, NULL, NULL ))
+ if (!(status = server_get_unix_fd( commio->hDevice, FILE_READ_DATA | FILE_WRITE_DATA, &fd, &needs_close, NULL, NULL )))
{
serial_irq_info new_irq_info;
DWORD new_mstat, new_evtmask;
@@ -962,31 +1002,60 @@ static DWORD CALLBACK wait_for_event(LPVOID arg)
* We don't handle the EV_RXFLAG (the eventchar)
*/
NtDelayExecution(FALSE, &time);
- get_irq_info(fd, &new_irq_info);
- if (get_modem_status(fd, &new_mstat))
- TRACE("get_modem_status failed\n");
- *commio->events = check_events(fd, commio->evtmask,
- &new_irq_info, &commio->irq_info,
- new_mstat, commio->mstat);
- if (*commio->events) break;
- get_wait_mask(commio->hDevice, &new_evtmask);
+ status = get_wait_mask(commio->hDevice, &new_evtmask);
+ if (status)
+ {
+ TRACE("get_wait_mask failed\n");
+ *commio->events = 0;
+ break;
+ }
if (commio->evtmask != new_evtmask)
{
*commio->events = 0;
break;
}
+ notimplemented = 0;
+ status = get_irq_info(fd, &new_irq_info);
+ if (status && status != STATUS_NOT_IMPLEMENTED)
+ {
+ TRACE("get_irq_status failed\n");
+ *commio->events = 0;
+ break;
+ }
+ status = get_modem_status(fd, &new_mstat);
+ if (status && status != STATUS_NOT_IMPLEMENTED)
+ {
+ TRACE("get_modem_status failed\n");
+ *commio->events = 0;
+ break;
+ }
+ notimplemented = 0;
+ status = check_events(fd, commio->evtmask,
+ &new_irq_info, &commio->irq_info,
+ new_mstat, commio->mstat, commio->events, ¬implemented);
+ if (status)
+ {
+ TRACE("check_events failed\n");
+ *commio->events = 0;
+ break;
+ }
+ if (*commio->events) break;
+ /* FIXME: check notimplemented */
}
if (needs_close) close( fd );
}
+ commio->piosb->u.Status = status;
+ commio->piosb->Information = sizeof(DWORD);
if (commio->hEvent) NtSetEvent(commio->hEvent, NULL);
RtlFreeHeap(GetProcessHeap(), 0, commio);
return 0;
}
-static NTSTATUS wait_on(HANDLE hDevice, int fd, HANDLE hEvent, DWORD* events)
+static NTSTATUS wait_on(HANDLE hDevice, int fd, HANDLE hEvent, DWORD* events, PIO_STATUS_BLOCK piosb)
{
async_commio* commio;
NTSTATUS status;
+ DWORD notimplemented = 0;
if ((status = NtResetEvent(hEvent, NULL)))
return status;
@@ -997,6 +1066,7 @@ static NTSTATUS wait_on(HANDLE hDevice, int fd, HANDLE hEvent, DWORD* events)
commio->hDevice = hDevice;
commio->events = events;
commio->hEvent = hEvent;
+ commio->piosb = piosb;
get_wait_mask(commio->hDevice, &commio->evtmask);
/* We may never return, if some capabilities miss
@@ -1034,6 +1104,7 @@ static NTSTATUS wait_on(HANDLE hDevice, int fd, HANDLE hEvent, DWORD* events)
FIXME("EV_RXFLAG not handled\n");
if ((status = get_irq_info(fd, &commio->irq_info)) &&
+ ! (status == STATUS_NOT_IMPLEMENTED && (commio->evtmask & (EV_RXCHAR|EV_TXEMPTY))) &&
(commio->evtmask & (EV_BREAK | EV_ERR)))
goto out_now;
@@ -1042,9 +1113,14 @@ static NTSTATUS wait_on(HANDLE hDevice, int fd, HANDLE hEvent, DWORD* events)
goto out_now;
/* We might have received something or the TX buffer is delivered */
- *events = check_events(fd, commio->evtmask,
- &commio->irq_info, &commio->irq_info,
- commio->mstat, commio->mstat);
+ status = check_events(fd, commio->evtmask,
+ &commio->irq_info, &commio->irq_info,
+ commio->mstat, commio->mstat,
+ events, ¬implemented);
+ if (status)
+ goto out_now;
+ if (notimplemented&(EV_RXCHAR|EV_TXEMPTY))
+ goto out_now;
if (*events)
{
status = STATUS_SUCCESS;
@@ -1052,6 +1128,8 @@ static NTSTATUS wait_on(HANDLE hDevice, int fd, HANDLE hEvent, DWORD* events)
}
/* create the worker for the task */
+ piosb->u.Status = STATUS_PENDING;
+ piosb->Information = sizeof(DWORD);
status = RtlQueueWorkItem(wait_for_event, commio, 0 /* FIXME */);
if (status != STATUS_SUCCESS) goto out_now;
return STATUS_PENDING;
@@ -1294,11 +1372,13 @@ static inline NTSTATUS io_control(HANDLE hDevice,
case IOCTL_SERIAL_WAIT_ON_MASK:
if (lpOutBuffer && nOutBufferSize == sizeof(DWORD))
{
- if (!(status = wait_on(hDevice, fd, hEvent, (DWORD*)lpOutBuffer)))
+ if (!(status = wait_on(hDevice, fd, hEvent, (DWORD*)lpOutBuffer, piosb)))
sz = sizeof(DWORD);
}
else
status = STATUS_INVALID_PARAMETER;
+ if (status == STATUS_PENDING)
+ return status;
break;
default:
FIXME("Unsupported IOCTL %x (type=%x access=%x func=%x meth=%x)\n",
@@ -1356,7 +1436,7 @@ NTSTATUS COMM_DeviceIoControl(HANDLE hDevice,
if (status == STATUS_PENDING)
{
NtWaitForSingleObject(hev, FALSE, NULL);
- status = STATUS_SUCCESS;
+ status = piosb->u.Status;
}
NtClose(hev);
}
--
1.5.6.5
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Leiter EDV
Leopoldstraße 15
80802 München
More information about the wine-patches
mailing list