serial: got a thread per IRP

winpr-comm: got IOCTL_SERIAL_GET_COMMSTATUS, IOCTL_SERIAL_SET_BREAK_ON and IOCTL_SERIAL_SET_BREAK_OFF
winpr-comm: tcdrain called by CommWriteFile() :(

channels/serial/client/serial_main.c

@@ -71,6 +71,8 @@ struct _SERIAL_DEVICE
 
 	/* one thread per pending IRP and indexed according their CompletionId */
 	wListDictionary *IrpThreads;
+	UINT32 IrpThreadToTerminateCount;
+	CRITICAL_SECTION TerminatingIrpThreadsLock;
 };
 
 typedef struct _IRP_THREAD_DATA IRP_THREAD_DATA;
@@ -180,8 +182,6 @@ static void serial_process_irp_create(SERIAL_DEVICE* serial, IRP* irp)
   error_handle:
 	Stream_Write_UINT32(irp->output, irp->FileId);	/* FileId (4 bytes) */
 	Stream_Write_UINT8(irp->output, 0);		/* Information (1 byte) */
-
-	irp->Complete(irp);
 }
 
 static void serial_process_irp_close(SERIAL_DEVICE* serial, IRP* irp)
@@ -202,8 +202,6 @@ static void serial_process_irp_close(SERIAL_DEVICE* serial, IRP* irp)
 
   error_handle:
 	Stream_Zero(irp->output, 5); /* Padding (5 bytes) */
-
-	irp->Complete(irp);
 }
 
 static void serial_process_irp_read(SERIAL_DEVICE* serial, IRP* irp)
@@ -289,8 +287,6 @@ static void serial_process_irp_read(SERIAL_DEVICE* serial, IRP* irp)
 
 	if (buffer)
 		free(buffer);
-
-	irp->Complete(irp);
 }
 
 static void serial_process_irp_write(SERIAL_DEVICE* serial, IRP* irp)
@@ -357,7 +353,6 @@ static void serial_process_irp_write(SERIAL_DEVICE* serial, IRP* irp)
 
 	Stream_Write_UINT32(irp->output, nbWritten); /* Length (4 bytes) */
 	Stream_Write_UINT8(irp->output, 0); /* Padding (1 byte) */
-	irp->Complete(irp);
 }
 
 
@@ -391,11 +386,13 @@ static void serial_process_irp_device_control(SERIAL_DEVICE* serial, IRP* irp)
 
 	Stream_Read(irp->input, InputBuffer, InputBufferLength);
 
-	DEBUG_SVC("CommDeviceIoControl: CompletionId=%d, IoControlCode=[0x%x] %s", irp->CompletionId, IoControlCode, _comm_serial_ioctl_name(IoControlCode));
+	DEBUG_SVC("CommDeviceIoControl: CompletionId=%d, IoControlCode=[0x%X] %s", irp->CompletionId, IoControlCode, _comm_serial_ioctl_name(IoControlCode));
 		
 	/* FIXME: CommDeviceIoControl to be replaced by DeviceIoControl() */
 	if (CommDeviceIoControl(serial->hComm, IoControlCode, InputBuffer, InputBufferLength, OutputBuffer, OutputBufferLength, &BytesReturned, NULL))
 	{
+		/* DEBUG_SVC("CommDeviceIoControl: CompletionId=%d, IoControlCode=[0x%X] %s done", irp->CompletionId, IoControlCode, _comm_serial_ioctl_name(IoControlCode)); */
+
 		irp->IoStatus = STATUS_SUCCESS;
 	}
 	else
@@ -457,7 +454,7 @@ static void serial_process_irp_device_control(SERIAL_DEVICE* serial, IRP* irp)
 		Stream_EnsureRemainingCapacity(irp->output, BytesReturned);
 		Stream_Write(irp->output, OutputBuffer, BytesReturned); /* OutputBuffer */
 	}
-	/* TMP: FIXME: Why at least Windows 2008R2 gets lost with this
+	/* FIXME: Why at least Windows 2008R2 gets lost with this
 	 * extra byte and likely on a IOCTL_SERIAL_SET_BAUD_RATE? The
 	 * extra byte is well required according MS-RDPEFS
 	 * 2.2.1.5.5 */
@@ -471,8 +468,6 @@ static void serial_process_irp_device_control(SERIAL_DEVICE* serial, IRP* irp)
 
 	if (OutputBuffer != NULL)
 		free(OutputBuffer);
-
-	irp->Complete(irp);
 }
 
 static void serial_process_irp(SERIAL_DEVICE* serial, IRP* irp)
@@ -505,11 +500,11 @@ static void serial_process_irp(SERIAL_DEVICE* serial, IRP* irp)
 		default:
 			DEBUG_WARN("MajorFunction 0x%X not supported", irp->MajorFunction);
 			irp->IoStatus = STATUS_NOT_SUPPORTED;
-			irp->Complete(irp);
 			break;
 	}
 }
 
+
 static void* irp_thread_func(void* arg)
 {
 	IRP_THREAD_DATA *data = (IRP_THREAD_DATA*)arg;
@@ -517,6 +512,17 @@ static void* irp_thread_func(void* arg)
 	/* blocks until the end of the request */
 	serial_process_irp(data->serial, data->irp);
 
+	EnterCriticalSection(&data->serial->TerminatingIrpThreadsLock);
+	data->serial->IrpThreadToTerminateCount++;
+
+	data->irp->Complete(data->irp);
+
+	LeaveCriticalSection(&data->serial->TerminatingIrpThreadsLock);
+
+	/* NB: At this point, the server might already being reusing
+	 * the CompletionId whereas the thread is not yet
+	 * terminated */
+
 	free(data);
 
 	ExitThread(0);
@@ -530,56 +536,95 @@ static void create_irp_thread(SERIAL_DEVICE *serial, IRP *irp)
 	HANDLE irpThread = INVALID_HANDLE_VALUE;
 	HANDLE previousIrpThread;
 
-	/* Checks whether a previous IRP with the same CompletionId
+	/* uncomment the code below to get a single thread per IRP for
-	 * was completed. NB: this can be the a recall of the same
+	 * a test/debug purpose. NB: two IRPs could not occur at the
-	 * request let as blocking. Behavior at least observed with
+	 * same time, typically two concurent Read/Write
-	 * IOCTL_SERIAL_WAIT_ON_MASK. FIXME: to be confirmed.
+	 * operations. */
-	 */
+	/* serial_process_irp(serial, irp); */
+	/* irp->Complete(irp); */
+	/* return; */
 
-	// TMP: there is a slight chance that the server sends a new request with the same CompletionId whereas the previous thread is not yet terminated.
 
-	previousIrpThread = ListDictionary_GetItemValue(serial->IrpThreads, (void*)irp->CompletionId);
+	EnterCriticalSection(&serial->TerminatingIrpThreadsLock);
-
+	while (serial->IrpThreadToTerminateCount > 0)
-	if (previousIrpThread) 
 	{
-		DWORD waitResult;
+		/* Cleaning up termitating and pending irp
+		 * threads. See also: irp_thread_func() */
 
-		/* FIXME: not quite sure a zero timeout is a good thing to check whether a thread is stil alived or not */
+		HANDLE irpThread;
-		waitResult = WaitForSingleObject(previousIrpThread, 0);
+		ULONG_PTR *ids;
+		int i, nbIds;
 
-		if (waitResult == WAIT_TIMEOUT)
+		nbIds = ListDictionary_GetKeys(serial->IrpThreads, &ids);
+		for (i=0; i<nbIds; i++)
 		{
-			/* Thread still alived */
+			/* Checking if ids[i] is terminating or pending */
-			/* FIXME: how to send a kind of wake up signal to accelerate the pending request */
 
-			DEBUG_WARN("IRP with the CompletionId=%d not yet completed!", irp->CompletionId);
+			DWORD waitResult;
+			ULONG_PTR id = ids[i];
 
-			// TMP:
+			irpThread = ListDictionary_GetItemValue(serial->IrpThreads, (void*)id);
-			assert(FALSE); /* assert() to be removed if it does realy happen */
+
-			
+			/* FIXME: not quite sure a zero timeout is a good thing to check whether a thread is stil alived or not */
-			/* FIXME: asserts that the previous thread's IRP is well the same request */
+			waitResult = WaitForSingleObject(irpThread, 0);
-			irp->Discard(irp);
+			if (waitResult == WAIT_OBJECT_0)
-			return;
+			{
+				/* terminating thread */
+
+				/* DEBUG_SVC("IRP thread with CompletionId=%d naturally died", id); */
+
+				CloseHandle(irpThread);
+				ListDictionary_Remove(serial->IrpThreads, (void*)id);
+
+				serial->IrpThreadToTerminateCount--;
+			}
+			else if (waitResult != WAIT_TIMEOUT)
+ 			{
+				/* unexpected thread state */
+
+				DEBUG_WARN("WaitForSingleObject, got an unexpected result=0x%X\n", waitResult);
+				assert(FALSE);
+			}
+			/* pending thread (but not yet terminating thread) if waitResult == WAIT_TIMEOUT */
 		}
-		else if(waitResult == WAIT_OBJECT_0)
+		
+
+		assert(serial->IrpThreadToTerminateCount == 0); /* TMP: */
+
+		if (serial->IrpThreadToTerminateCount > 0)
 		{
-			DEBUG_SVC("previous IRP thread with CompletionId=%d naturally died", irp->CompletionId);
+			DEBUG_SVC("%d IRP thread(s) not yet terminated", serial->IrpThreadToTerminateCount);
-
+			Sleep(1); /* 1 ms */
-			/* the previous thread naturally died */
-			CloseHandle(previousIrpThread);
-			ListDictionary_Remove(serial->IrpThreads, (void*)irp->CompletionId);
-		}
-		else
-		{
-			/* FIXME: handle more error cases */
-			DEBUG_WARN("IRP CompletionId=%d : unexpected waitResult=%X");
-			irp->Discard(irp);
-
-			assert(FALSE); /* should not happen */
-
-			return;
 		}
 	}
+	LeaveCriticalSection(&serial->TerminatingIrpThreadsLock);
+
+	/* NB: At this point and thanks to the synchronization we're
+	 * sure that the incoming IRP uses well a recycled
+	 * CompletionId or the server sent again an IRP already posted
+	 * which didn't get yet a response (this later server behavior
+	 * at least observed with IOCTL_SERIAL_WAIT_ON_MASK FIXME:
+	 * behavior documented somewhere?).
+	 */
+
+	previousIrpThread = ListDictionary_GetItemValue(serial->IrpThreads, (void*)irp->CompletionId);
+	if (previousIrpThread) 
+	{
+		/* Thread still alived <=> Request still pending */
+
+		DEBUG_SVC("IRP recall: IRP with the CompletionId=%d not yet completed!", irp->CompletionId);
+
+		/* TMP: TODO: taking over the pending IRP or sending a kind of wake up signal to accelerate the pending request */
+		assert(FALSE);
+			
+		/* FIXME: asserts that the previous thread's IRP is
+		 * well the same request by checking more
+		 * details. Need an access to the IRP object used by
+		 * previousIrpThread */
+		irp->Discard(irp);
+		return;
+	}
+
 
 	if (ListDictionary_Count(serial->IrpThreads) >= MAX_IRP_THREADS)
 	{
@@ -589,6 +634,7 @@ static void create_irp_thread(SERIAL_DEVICE *serial, IRP *irp)
 		/* TODO: FIXME: WaitForMultipleObjects() not yet implemented for threads */
 	}
 
+
 	/* error_handle to be used ... */
 
 	data = (IRP_THREAD_DATA*)calloc(1, sizeof(IRP_THREAD_DATA));
@@ -692,6 +738,7 @@ static void serial_free(DEVICE* device)
 	Stream_Free(serial->device.data, TRUE);
 	MessageQueue_Free(serial->MainIrpQueue);
 	ListDictionary_Free(serial->IrpThreads);
+	DeleteCriticalSection(&serial->TerminatingIrpThreadsLock);
 
 	free(serial);
 }
@@ -747,6 +794,9 @@ int DeviceServiceEntry(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints)
 
 		serial->IrpThreads = ListDictionary_New(FALSE); /* only handled in create_irp_thread() */
 
+		serial->IrpThreadToTerminateCount = 0;
+		InitializeCriticalSection(&serial->TerminatingIrpThreadsLock);
+
 		WLog_Init();
 		serial->log = WLog_Get("com.freerdp.channel.serial.client");
 		WLog_Print(serial->log, WLOG_DEBUG, "initializing");

winpr/include/winpr/comm.h

@@ -2,6 +2,8 @@
  * WinPR: Windows Portable Runtime
  * Serial Communication API
  *
+ * Copyright 2011 O.S. Systems Software Ltda.
+ * Copyright 2011 Eduardo Fiss Beloni <beloni@ossystems.com.br>
  * Copyright 2014 Marc-Andre Moreau <marcandre.moreau@gmail.com>
  * Copyright 2014 Hewlett-Packard Development Company, L.P.
  *
@@ -420,8 +422,8 @@ WINPR_API HANDLE CommCreateFileA(LPCSTR lpFileName, DWORD dwDesiredAccess, DWORD
 #define IOCTL_SERIAL_CLR_RTS		0x001B0034
 /* IOCTL_SERIAL_SET_XOFF 0x001B0038 */
 /* IOCTL_SERIAL_SET_XON 0x001B003C */
-/* IOCTL_SERIAL_SET_BREAK_ON 0x001B0010 */
+#define IOCTL_SERIAL_SET_BREAK_ON	0x001B0010
-/* IOCTL_SERIAL_SET_BREAK_OFF 0x001B0014 */
+#define IOCTL_SERIAL_SET_BREAK_OFF	0x001B0014
 #define IOCTL_SERIAL_SET_QUEUE_SIZE	0x001B0008
 #define IOCTL_SERIAL_GET_WAIT_MASK	0x001B0040
 #define IOCTL_SERIAL_SET_WAIT_MASK	0x001B0044
@@ -432,7 +434,10 @@ WINPR_API HANDLE CommCreateFileA(LPCSTR lpFileName, DWORD dwDesiredAccess, DWORD
 #define IOCTL_SERIAL_SET_HANDFLOW	0x001B0064
 #define IOCTL_SERIAL_GET_MODEMSTATUS	0x001B0068
 /* IOCTL_SERIAL_GET_DTRRTS 0x001B0078 */
-/* IOCTL_SERIAL_GET_COMMSTATUS 0x001B0084 */
+
+/* according to [MS-RDPESP] it should be 0x001B0084, but servers send 0x001B006C */
+#define IOCTL_SERIAL_GET_COMMSTATUS	0x001B006C
+
 #define IOCTL_SERIAL_GET_PROPERTIES	0x001B0074
 /* IOCTL_SERIAL_XOFF_COUNTER 0x001B0070 */
 /* IOCTL_SERIAL_LSRMST_INSERT 0x001B007C */
@@ -480,8 +485,8 @@ static const _SERIAL_IOCTL_NAME _SERIAL_IOCTL_NAMES[] =
 	{IOCTL_SERIAL_CLR_RTS,		"IOCTL_SERIAL_CLR_RTS"},
 	// {IOCTL_SERIAL_SET_XOFF,		"IOCTL_SERIAL_SET_XOFF"},
 	// {IOCTL_SERIAL_SET_XON,		"IOCTL_SERIAL_SET_XON"},
-	// {IOCTL_SERIAL_SET_BREAK_ON,  	"IOCTL_SERIAL_SET_BREAK_ON"},
+	{IOCTL_SERIAL_SET_BREAK_ON,  	"IOCTL_SERIAL_SET_BREAK_ON"},
-	// {IOCTL_SERIAL_SET_BREAK_OFF,	"IOCTL_SERIAL_SET_BREAK_OFF"},
+	{IOCTL_SERIAL_SET_BREAK_OFF,	"IOCTL_SERIAL_SET_BREAK_OFF"},
 	{IOCTL_SERIAL_SET_QUEUE_SIZE, 	"IOCTL_SERIAL_SET_QUEUE_SIZE"},
 	{IOCTL_SERIAL_GET_WAIT_MASK,	"IOCTL_SERIAL_GET_WAIT_MASK"},
 	{IOCTL_SERIAL_SET_WAIT_MASK,	"IOCTL_SERIAL_SET_WAIT_MASK"},
@@ -492,7 +497,7 @@ static const _SERIAL_IOCTL_NAME _SERIAL_IOCTL_NAMES[] =
 	{IOCTL_SERIAL_SET_HANDFLOW,	"IOCTL_SERIAL_SET_HANDFLOW"},
 	{IOCTL_SERIAL_GET_MODEMSTATUS,	"IOCTL_SERIAL_GET_MODEMSTATUS"},
 	// {IOCTL_SERIAL_GET_DTRRTS,	"IOCTL_SERIAL_GET_DTRRTS"},
-	// {IOCTL_SERIAL_GET_COMMSTATUS,	"IOCTL_SERIAL_GET_COMMSTATUS"},
+	{IOCTL_SERIAL_GET_COMMSTATUS,	"IOCTL_SERIAL_GET_COMMSTATUS"},
 	{IOCTL_SERIAL_GET_PROPERTIES,	"IOCTL_SERIAL_GET_PROPERTIES"},
 	// {IOCTL_SERIAL_XOFF_COUNTER,	"IOCTL_SERIAL_XOFF_COUNTER"},
 	// {IOCTL_SERIAL_LSRMST_INSERT,	"IOCTL_SERIAL_LSRMST_INSERT"},

winpr/libwinpr/comm/comm.h

@@ -56,7 +56,6 @@ struct winpr_comm
 	 */
 	BOOL permissive;
 
-
 	// TMP: to be renamed serverSerialDriverId
 	REMOTE_SERIAL_DRIVER_ID remoteSerialDriverId;
 
@@ -66,9 +65,14 @@ struct winpr_comm
 
 	COMMTIMEOUTS timeouts;
 	
+	/* NB: no synchronization required on counters until _get_commstatus()
+	 * is the only function [except CreateFile() and CloseHandle()] to
+	 * modify counters */
 	struct serial_icounter_struct counters;
+
+	/* TMP: TODO: sync */
 	ULONG waitMask; /* TMP: to be renamed EventMask */
-	ULONG pendingEvents;
+	ULONG PendingEvents;
 
 	/* NB: CloseHandle() has to free resources */
 };

winpr/libwinpr/comm/comm_io.c

@@ -326,7 +326,19 @@ BOOL CommWriteFile(HANDLE hDevice, LPCVOID lpBuffer, DWORD nNumberOfBytesToWrite
 
 		*lpNumberOfBytesWritten += nbWritten;
 	}
-	
+
+
+	/* FIXME: this call to tcdrain() doesn't look correct and
+	 * might hide a bug but was required while testing a serial
+	 * printer. Its driver was expecting the modem line status
+	 * SERIAL_MSR_DSR true after the sending which was never
+	 * happenning otherwise. A purge was also done before each
+	 * Write operation. The serial port was oppened with:
+	 * DesiredAccess=0x0012019F. The printer worked fine with
+	 * mstsc. */
+	tcdrain(pComm->fd);
+
+
 	return TRUE;
 }
 

winpr/libwinpr/comm/comm_ioctl.c

@@ -139,7 +139,6 @@ static BOOL _CommDeviceIoControl(HANDLE hDevice, DWORD dwIoControlCode, LPVOID l
 	switch (dwIoControlCode)
 	{
 		case 0x220034:
-		case 0X1B006C:
 			DEBUG_WARN("Undocumented IoControlCode: 0X%X", dwIoControlCode);
 			*lpBytesReturned = nOutBufferSize; /* an empty OutputBuffer will be returned */
 			SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
@@ -509,7 +508,43 @@ static BOOL _CommDeviceIoControl(HANDLE hDevice, DWORD dwIoControlCode, LPVOID l
 			}
 			break;
 		}
+		case IOCTL_SERIAL_GET_COMMSTATUS:
+		{
+			if (pRemoteSerialDriver->get_commstatus)
+			{
+				SERIAL_STATUS *pCommstatus = (SERIAL_STATUS*)lpOutBuffer;
+				
+				assert(nOutBufferSize >= sizeof(SERIAL_STATUS));
+				if (nOutBufferSize < sizeof(SERIAL_STATUS))
+				{
+					SetLastError(ERROR_INSUFFICIENT_BUFFER);
+					return FALSE;					
+				}
 
+				if (!pRemoteSerialDriver->get_commstatus(pComm, pCommstatus))
+					return FALSE;
+
+				*lpBytesReturned = sizeof(SERIAL_STATUS);
+				return TRUE;
+			}
+			break;
+		}
+		case IOCTL_SERIAL_SET_BREAK_ON:
+		{
+			if (pRemoteSerialDriver->set_break_on)
+			{
+				return pRemoteSerialDriver->set_break_on(pComm);
+			}
+			break;
+		}
+		case IOCTL_SERIAL_SET_BREAK_OFF:
+		{
+			if (pRemoteSerialDriver->set_break_off)
+			{
+				return pRemoteSerialDriver->set_break_off(pComm);
+			}
+			break;
+		}
 	}
 	
 	DEBUG_WARN(_T("unsupported IoControlCode=[0x%lX] %s (remote serial driver: %s)"), 

winpr/libwinpr/comm/comm_ioctl.h

@@ -175,6 +175,30 @@ typedef struct _SERIAL_QUEUE_SIZE
 #define SERIAL_PURGE_TXCLEAR 0x00000004 
 #define SERIAL_PURGE_RXCLEAR 0x00000008 
 
+typedef struct _SERIAL_STATUS
+{ 
+	ULONG Errors; 
+	ULONG HoldReasons; 
+	ULONG AmountInInQueue; 
+	ULONG AmountInOutQueue; 
+	BOOLEAN EofReceived;
+	BOOLEAN WaitForImmediate; 
+} SERIAL_STATUS, *PSERIAL_STATUS; 
+
+#define SERIAL_TX_WAITING_FOR_CTS      ((ULONG)0x00000001) 
+#define SERIAL_TX_WAITING_FOR_DSR      ((ULONG)0x00000002) 
+#define SERIAL_TX_WAITING_FOR_DCD      ((ULONG)0x00000004) 
+#define SERIAL_TX_WAITING_FOR_XON      ((ULONG)0x00000008) 
+#define SERIAL_TX_WAITING_XOFF_SENT    ((ULONG)0x00000010) 
+#define SERIAL_TX_WAITING_ON_BREAK     ((ULONG)0x00000020) 
+#define SERIAL_RX_WAITING_FOR_DSR      ((ULONG)0x00000040) 
+ 
+#define SERIAL_ERROR_BREAK             ((ULONG)0x00000001) 
+#define SERIAL_ERROR_FRAMING           ((ULONG)0x00000002) 
+#define SERIAL_ERROR_OVERRUN           ((ULONG)0x00000004) 
+#define SERIAL_ERROR_QUEUEOVERRUN      ((ULONG)0x00000008) 
+#define SERIAL_ERROR_PARITY            ((ULONG)0x00000010) 
+
 /**
  * A function might be NULL if not supported by the underlying remote driver.
  *
@@ -205,6 +229,9 @@ typedef struct _REMOTE_SERIAL_DRIVER
 	BOOL (*wait_on_mask)(WINPR_COMM *pComm, ULONG *pOutputMask);
 	BOOL (*set_queue_size)(WINPR_COMM *pComm, const SERIAL_QUEUE_SIZE *pQueueSize);
 	BOOL (*purge)(WINPR_COMM *pComm, const ULONG *pPurgeMask);
+	BOOL (*get_commstatus)(WINPR_COMM *pComm, SERIAL_STATUS *pCommstatus);
+	BOOL (*set_break_on)(WINPR_COMM *pComm);
+	BOOL (*set_break_off)(WINPR_COMM *pComm);
 
 } REMOTE_SERIAL_DRIVER;
 

winpr/libwinpr/comm/comm_sercx2_sys.c

@@ -145,6 +145,9 @@ static REMOTE_SERIAL_DRIVER _SerCx2Sys =
 	.wait_on_mask     = NULL,
 	.set_queue_size   = NULL,
 	.purge            = _purge,
+	.get_commstatus   = NULL,
+	.set_break_on     = NULL,
+	.set_break_off    = NULL,
 };
 
 
@@ -185,6 +188,10 @@ REMOTE_SERIAL_DRIVER* SerCx2Sys_s()
 
 	_SerCx2Sys.set_queue_size = pSerialSys->set_queue_size;
 
+	_SerCx2Sys.get_commstatus = pSerialSys->get_commstatus;
+
+	_SerCx2Sys.set_break_on  = pSerialSys->set_break_on;
+	_SerCx2Sys.set_break_off = pSerialSys->set_break_off;
 
 	return &_SerCx2Sys;
 }

winpr/libwinpr/comm/comm_sercx_sys.c

@@ -394,6 +394,9 @@ static REMOTE_SERIAL_DRIVER _SerCxSys =
 	.wait_on_mask     = NULL,
 	.set_queue_size   = NULL,
 	.purge            = NULL,
+	.get_commstatus   = NULL,
+	.set_break_on     = NULL,
+	.set_break_off    = NULL,
 };
 
 
@@ -427,6 +430,11 @@ REMOTE_SERIAL_DRIVER* SerCxSys_s()
 
 	_SerCxSys.purge = pSerialSys->purge;
 
+	_SerCxSys.get_commstatus = pSerialSys->get_commstatus;
+
+	_SerCxSys.set_break_on  = pSerialSys->set_break_on;
+	_SerCxSys.set_break_off = pSerialSys->set_break_off;
+
 	return &_SerCxSys;
 }
 

winpr/libwinpr/comm/comm_serial_sys.c

@@ -967,10 +967,12 @@ static BOOL _get_modemstatus(WINPR_COMM *pComm, ULONG *pRegister)
 	}
 
 	ZeroMemory(pRegister, sizeof(ULONG));
-
+	
 	/* FIXME: Is the last read of the MSR register available or
 	 * cached somewhere? Not quite sure we need to return the 4
-	 * LSBits anyway.
+	 * LSBits anyway. A direct access to the register -- which
+	 * would reset the register -- is likely not expected from
+	 * this function.
 	 */
 
 	/* #define SERIAL_MSR_DCTS     0x01 */
@@ -1024,7 +1026,7 @@ static BOOL _set_wait_mask(WINPR_COMM *pComm, const ULONG *pWaitMask)
 			return FALSE;
 		}
 
-		pComm->pendingEvents = 0;
+		pComm->PendingEvents = 0;
 	}
 	
 	// TMP: TODO:
@@ -1055,201 +1057,6 @@ static BOOL _get_wait_mask(WINPR_COMM *pComm, ULONG *pWaitMask)
 }
 
 
-static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
-{
-	assert(*pOutputMask == 0);
-
-	// TMP: TODO: be sure to get a dedicated thread
-	/* while (TRUE) */
-	{
-		int nbBytesToBeRead = 0;
-		int nbBytesToBeWritten = 0;
-		struct serial_icounter_struct currentCounters; 
-		ULONG tiocmiwaitMask = 0; /* TIOCMIWAIT can wait for the 4 lines: TIOCM_RNG/DSR/CD/CTS */
-
-		if (ioctl(pComm->fd, TIOCINQ, &nbBytesToBeRead) < 0)
-		{
-			DEBUG_WARN("TIOCINQ ioctl failed, errno=[%d] %s", errno, strerror(errno));
-			SetLastError(ERROR_IO_DEVICE);
-			return FALSE;
-		}
-
-		if (ioctl(pComm->fd, TIOCOUTQ, &nbBytesToBeWritten) < 0)
-		{
-			DEBUG_WARN("TIOCOUTQ ioctl failed, errno=[%d] %s", errno, strerror(errno));
-			SetLastError(ERROR_IO_DEVICE);
-			return FALSE;
-		}
-
-		ZeroMemory(&currentCounters, sizeof(struct serial_icounter_struct));
-		if (ioctl(pComm->fd, TIOCGICOUNT, &currentCounters) < 0)
-		{
-			DEBUG_WARN("TIOCGICOUNT ioctl failed, errno=[%d] %s", errno, strerror(errno));
-			SetLastError(ERROR_IO_DEVICE);
-			return FALSE;
-		}
-
-		/* NB: preferred below (currentCounters.* != pComm->counters.*) over (currentCounters.* > pComm->counters.*) thinking the counters can loop */
-
-
-		/* events */
-
-		if (pComm->waitMask & SERIAL_EV_RXCHAR)
-		{
-			if (nbBytesToBeRead > 0)
-			{
-				/* at least one character is pending to be read */
-				*pOutputMask |= SERIAL_EV_RXCHAR;
-			}
-		}
-		
-		if (pComm->waitMask & SERIAL_EV_RXFLAG)
-		{
-			if (pComm->pendingEvents & SERIAL_EV_RXFLAG) // TMP: to be done in the ReadThread
-			{
-				/* the event character was received FIXME: is the character supposed to be still in the input buffer? */
-				
-				/* event consumption */
-				pComm->pendingEvents &= ~SERIAL_EV_RXFLAG;
-				*pOutputMask |= SERIAL_EV_RXFLAG;
-			}
-		}
-
-		if (pComm->waitMask & SERIAL_EV_TXEMPTY)
-		{
-			if (nbBytesToBeWritten == 0)
-			{
-				/* NB: as of today CommWriteFile still blocks and uses the same thread than CommDeviceIoControl, 
-				 *  it should be enough to just check nbBytesToBeWritten 
-				 */
-
-				/* the output buffer is empty */
-				*pOutputMask |= SERIAL_EV_TXEMPTY; 
-			}
-		}
-
-		if (pComm->waitMask & SERIAL_EV_CTS)
-		{
-			tiocmiwaitMask |= TIOCM_CTS;
-		}
-
-		if (pComm->waitMask & SERIAL_EV_DSR)
-		{
-			tiocmiwaitMask |= TIOCM_DSR;
-		}
-
-		if (pComm->waitMask & SERIAL_EV_RLSD)
-		{
-			tiocmiwaitMask |= TIOCM_CD;
-		}
-
-		if (pComm->waitMask & SERIAL_EV_BREAK)
-		{
-			if (currentCounters.brk != pComm->counters.brk)
-			{
-				*pOutputMask |= SERIAL_EV_BREAK;
-
-				/* event consumption */
-				pComm->counters.brk = currentCounters.brk;
-			}
-		}
-			 
-		if (pComm->waitMask & SERIAL_EV_ERR)
-		{
-			if ((currentCounters.frame != pComm->counters.frame) ||
-			    (currentCounters.overrun != pComm->counters.overrun) ||
-			    (currentCounters.parity != pComm->counters.parity))
-			{
-				*pOutputMask |= SERIAL_EV_ERR;
-
-				/* event consumption */
-				pComm->counters.frame = currentCounters.frame;
-				pComm->counters.overrun = currentCounters.overrun;
-				pComm->counters.parity = currentCounters.parity;
-			}
-		}
-
-		if (pComm->waitMask & SERIAL_EV_RING)
-		{
-			tiocmiwaitMask |= TIOCM_RNG;
-		}
-
-		if (pComm->waitMask & SERIAL_EV_RX80FULL)
-		{
-			if (nbBytesToBeRead > (0.8 * N_TTY_BUF_SIZE))
-				*pOutputMask |= SERIAL_EV_RX80FULL;
-		}
-
-		if ((*pOutputMask == 0) && /* don't need to wait more if at least an event already occured */
-		    (tiocmiwaitMask > 0))
-		{
-			if ((pComm->waitMask & SERIAL_EV_CTS) && currentCounters.cts != pComm->counters.cts)
-			{
-				*pOutputMask |= SERIAL_EV_CTS;
-
-				/* event consumption */
-				pComm->counters.cts = currentCounters.cts;
-			}
-
-			if ((pComm->waitMask & SERIAL_EV_DSR) && currentCounters.dsr != pComm->counters.dsr)
-			{
-				*pOutputMask |= SERIAL_EV_DSR;
-
-				/* event consumption */
-				pComm->counters.dsr = currentCounters.dsr;
-			}
-
-			if ((pComm->waitMask & SERIAL_EV_RLSD) && currentCounters.dcd != pComm->counters.dcd)
-			{
-				*pOutputMask |= SERIAL_EV_RLSD;
-
-				/* event consumption */
-				pComm->counters.dcd = currentCounters.dcd;
-			}
-
-			if ((pComm->waitMask & SERIAL_EV_RING) && currentCounters.rng != pComm->counters.rng)
-			{
-				*pOutputMask |= SERIAL_EV_RING;
-
-				/* event consumption */
-				pComm->counters.rng = currentCounters.rng;
-			}
-
-
-			// TMP: TIOCMIWAIT could be possible if _wait_on_mask gets its own thread 
-			/* if ((*pOutputMask == 0) && /\* don't bother at least one of the events event already occured *\/ */
-			/*     ((pComm->waitMask & ~(SERIAL_EV_CTS | SERIAL_EV_DSR | SERIAL_EV_RLSD | SERIAL_EV_RING)) == 0)) /\* only events handled by TIOCMIWAIT, otherwise go through the regular loop *\/ */
-			/* { */
-			/* 	if (ioctl(pComm->fd, TIOCMIWAIT, &tiocmiwaitMask) < 0) */
-			/* 	{ */
-			/* 		DEBUG_WARN("TIOCMIWAIT ioctl failed, errno=[%d] %s", errno, strerror(errno)); */
-			/* 		SetLastError(ERROR_IO_DEVICE); */
-			/* 		return FALSE; */
-			/* 	} */
-				
-			/* 	/\* check counters again after TIOCMIWAIT *\/ */
-			/* 	continue; */
-			/* } */
-		}
-
-		if (*pOutputMask != 0)
-		{
-			/* at least an event occurred */
-			return TRUE;
-		}
-
-		/* /\* // TMP: *\/ */
-		/* DEBUG_WARN("waiting on events:0X%lX", pComm->waitMask); */
-
-		/* sleep(1); */
-		
-	}
-
-	DEBUG_WARN("_wait_on_mask pending on events:0X%lX", pComm->waitMask);
-	SetLastError(ERROR_IO_PENDING); /* see: WaitCommEvent's help */
-	return FALSE;
-}
-
 
 static BOOL _set_queue_size(WINPR_COMM *pComm, const SERIAL_QUEUE_SIZE *pQueueSize)
 {
@@ -1289,7 +1096,7 @@ static BOOL _purge(WINPR_COMM *pComm, const ULONG *pPurgeMask)
 		SetLastError(ERROR_INVALID_PARAMETER);
 		return FALSE;
 	}
-
+	
 	/* FIXME: don't rely so much on how the IRP queues are implemented, should be more generic */
 
 	/* nothing to do until IRP_MJ_WRITE-s and IRP_MJ_DEVICE_CONTROL-s are executed in the same thread */
@@ -1303,15 +1110,16 @@ static BOOL _purge(WINPR_COMM *pComm, const ULONG *pPurgeMask)
 	{
 		/* Purges all read (IRP_MJ_READ) requests. */
 
+		// TMP:
 		if (pComm->ReadIrpQueue != NULL)
 		{
+			assert(0);
 			MessageQueue_Clear(pComm->ReadIrpQueue);
 		}
 
 		/* TMP: TODO: double check if this gives well a change to abort a pending CommReadFile */
-		//assert(0);
+		/* assert(0); */
 		/* fcntl(pComm->fd, F_SETFL, fcntl(pComm->fd, F_GETFL) | O_NONBLOCK); */
-		/* sleep(1); */
 		/* fcntl(pComm->fd, F_SETFL, fcntl(pComm->fd, F_GETFL) & ~O_NONBLOCK); */
 
 		/* TMP: FIXME: synchronization of the incoming
@@ -1348,6 +1156,222 @@ static BOOL _purge(WINPR_COMM *pComm, const ULONG *pPurgeMask)
 	return TRUE;
 }
 
+/* NB: _get_commstatus also produces most of the events consumed by _wait_on_mask(). Exceptions:
+ *  - SERIAL_EV_RXFLAG: FIXME: once EventChar supported
+ *
+ */
+static BOOL _get_commstatus(WINPR_COMM *pComm, SERIAL_STATUS *pCommstatus)
+{
+	/* http://msdn.microsoft.com/en-us/library/jj673022%28v=vs.85%29.aspx */
+
+	struct serial_icounter_struct currentCounters; 
+
+	ZeroMemory(pCommstatus, sizeof(SERIAL_STATUS));
+
+	ZeroMemory(&currentCounters, sizeof(struct serial_icounter_struct));
+	if (ioctl(pComm->fd, TIOCGICOUNT, &currentCounters) < 0)
+	{
+		DEBUG_WARN("TIOCGICOUNT ioctl failed, errno=[%d] %s", errno, strerror(errno));
+		SetLastError(ERROR_IO_DEVICE);
+		return FALSE;
+	}
+
+	/* NB: preferred below (currentCounters.* != pComm->counters.*) over (currentCounters.* > pComm->counters.*) thinking the counters can loop */
+
+	/* Errors */
+
+	if (currentCounters.buf_overrun != pComm->counters.buf_overrun)
+	{
+		pCommstatus->Errors |= SERIAL_ERROR_QUEUEOVERRUN;
+	}
+
+	if (currentCounters.overrun != pComm->counters.overrun)
+	{
+		pCommstatus->Errors |= SERIAL_ERROR_OVERRUN;
+		pComm->PendingEvents |= SERIAL_EV_ERR;
+	}
+
+	if (currentCounters.brk != pComm->counters.brk)
+	{
+		pCommstatus->Errors |= SERIAL_ERROR_BREAK;
+		pComm->PendingEvents |= SERIAL_EV_BREAK;
+	}
+
+	if (currentCounters.parity != pComm->counters.parity)
+	{
+		pCommstatus->Errors |= SERIAL_ERROR_PARITY;
+		pComm->PendingEvents |= SERIAL_EV_ERR;
+	}
+
+	if (currentCounters.frame != pComm->counters.frame)
+	{
+		pCommstatus->Errors |= SERIAL_ERROR_FRAMING;
+		pComm->PendingEvents |= SERIAL_EV_ERR;
+	}
+
+
+	/* HoldReasons TMP: TODO: see also _set_lines(), _clear_lines() the LCR register. */
+
+	/* AmountInInQueue */
+
+	if (ioctl(pComm->fd, TIOCINQ, &(pCommstatus->AmountInInQueue)) < 0)
+	{
+		DEBUG_WARN("TIOCINQ ioctl failed, errno=[%d] %s", errno, strerror(errno));
+		SetLastError(ERROR_IO_DEVICE);
+		return FALSE;
+	}
+
+
+	/*  AmountInOutQueue */
+
+	if (ioctl(pComm->fd, TIOCOUTQ, &(pCommstatus->AmountInOutQueue)) < 0)
+	{
+		DEBUG_WARN("TIOCOUTQ ioctl failed, errno=[%d] %s", errno, strerror(errno));
+		SetLastError(ERROR_IO_DEVICE);
+		return FALSE;
+	}
+
+	/*  BOOLEAN EofReceived; FIXME: once EofChar supported */
+
+
+	/*  BOOLEAN WaitForImmediate; TMP: TODO: once IOCTL_SERIAL_IMMEDIATE_CHAR supported */
+		
+
+	/* other events based on counters */
+
+	if (currentCounters.rx != pComm->counters.rx)
+	{
+		pComm->PendingEvents |= SERIAL_EV_RXCHAR;
+	}
+
+	if ((currentCounters.tx != pComm->counters.tx) && /* at least a transmission occurred AND ...*/
+	    (pCommstatus->AmountInOutQueue == 0)) /* output bufer is now empty */
+	{
+		pComm->PendingEvents |= SERIAL_EV_TXEMPTY;
+	}
+	else
+	{
+		/* FIXME: "now empty" is ambiguous, need to track previous completed transmission? */
+		pComm->PendingEvents &= ~SERIAL_EV_TXEMPTY;
+	}
+
+	if (currentCounters.cts != pComm->counters.cts)
+	{
+		pComm->PendingEvents |= SERIAL_EV_CTS;
+	}
+
+	if (currentCounters.dsr != pComm->counters.dsr)
+	{
+		pComm->PendingEvents |= SERIAL_EV_DSR;
+	}
+
+	if (currentCounters.dcd != pComm->counters.dcd)
+	{
+		pComm->PendingEvents |= SERIAL_EV_RLSD;
+	}
+
+	if (currentCounters.rng != pComm->counters.rng)
+	{
+		pComm->PendingEvents |= SERIAL_EV_RING;
+	}
+
+	if (pCommstatus->AmountInInQueue > (0.8 * N_TTY_BUF_SIZE))
+	{
+		pComm->PendingEvents |= SERIAL_EV_RX80FULL;
+	}
+	else
+	{
+		/* FIXME: "is 80 percent full" is ambiguous, need to track when it previously occured? */
+		pComm->PendingEvents &= ~SERIAL_EV_RX80FULL;
+	}
+
+
+	pComm->counters = currentCounters;
+
+
+	return TRUE;
+}
+
+static void _consume_event(WINPR_COMM *pComm, ULONG *pOutputMask, ULONG event)
+{
+	if ((pComm->waitMask & event) && (pComm->PendingEvents & event))
+	{
+		pComm->PendingEvents &= ~event; /* consumed */
+		*pOutputMask |= event;
+	}
+}
+
+static BOOL _wait_on_mask(WINPR_COMM *pComm, ULONG *pOutputMask)
+{
+	assert(*pOutputMask == 0);
+
+
+	while (TRUE)
+	{
+		SERIAL_STATUS serialStatus;
+
+		/* NB: also ensures PendingEvents to be up to date */
+		ZeroMemory(&serialStatus, sizeof(SERIAL_STATUS));
+		if (!_get_commstatus(pComm, &serialStatus))
+		{
+			return FALSE;
+		}
+
+		/* events */
+
+		_consume_event(pComm, pOutputMask, SERIAL_EV_RXCHAR);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_RXFLAG);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_TXEMPTY);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_CTS);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_DSR);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_RLSD);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_BREAK);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_ERR);
+		_consume_event(pComm, pOutputMask, SERIAL_EV_RING );
+		_consume_event(pComm, pOutputMask, SERIAL_EV_RX80FULL);
+
+		if (*pOutputMask != 0)
+		{
+			/* at least an event occurred */
+			return TRUE;
+		}
+
+		/* // TMP: */
+		DEBUG_WARN("waiting on events:0X%lX", pComm->waitMask);
+
+		sleep(1); // TMP: TODO: wait also on a PendingEvents modification, and a new identical IRP
+	}
+
+	DEBUG_WARN("_wait_on_mask pending on events:0X%lX", pComm->waitMask);
+	SetLastError(ERROR_IO_PENDING); /* see: WaitCommEvent's help */
+	return FALSE;
+}
+
+static BOOL _set_break_on(WINPR_COMM *pComm)
+{
+	if (ioctl(pComm->fd, TIOCSBRK, NULL) < 0)
+	{
+		DEBUG_WARN("TIOCSBRK ioctl failed, errno=[%d] %s", errno, strerror(errno));
+		SetLastError(ERROR_IO_DEVICE);
+		return FALSE;
+	}
+
+	return TRUE;
+}
+	
+
+static BOOL _set_break_off(WINPR_COMM *pComm)
+{
+	if (ioctl(pComm->fd, TIOCCBRK, NULL) < 0)
+	{
+		DEBUG_WARN("TIOCSBRK ioctl failed, errno=[%d] %s", errno, strerror(errno));
+		SetLastError(ERROR_IO_DEVICE);
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 
 static REMOTE_SERIAL_DRIVER _SerialSys = 
 {
@@ -1374,6 +1398,9 @@ static REMOTE_SERIAL_DRIVER _SerialSys =
 	.wait_on_mask     = _wait_on_mask,
 	.set_queue_size   = _set_queue_size,
 	.purge            = _purge,
+	.get_commstatus   = _get_commstatus,
+	.set_break_on     = _set_break_on,
+	.set_break_off    = _set_break_off,
 };