channels/serial: code cleanup

2026-04-15 00:44:19 +09:00 · 2014-03-26 17:11:15 -04:00
parent 2ff5200f81
commit 8e2baab504
2 changed files with 83 additions and 71 deletions
--- a/channels/serial/client/serial_main.c
+++ b/channels/serial/client/serial_main.c
@@ -45,6 +45,7 @@
 #include "serial_constants.h"

 #include <winpr/crt.h>
+#include <winpr/wlog.h>
 #include <winpr/synch.h>
 #include <winpr/thread.h>
 #include <winpr/stream.h>
@@ -64,6 +65,7 @@ struct _SERIAL_DEVICE
 	char* path;
 	SERIAL_TTY* tty;

+	wLog* log;
 	HANDLE thread;
 	wMessageQueue* IrpQueue;
 };
@@ -73,8 +75,8 @@ static void serial_process_irp_create(SERIAL_DEVICE* serial, IRP* irp)
 	int status;
 	UINT32 FileId;
 	UINT32 PathLength;
-	SERIAL_TTY* tty;
 	char* path = NULL;
+	SERIAL_TTY* tty;

 	Stream_Seek_UINT32(irp->input); /* DesiredAccess (4 bytes) */
 	Stream_Seek_UINT64(irp->input); /* AllocationSize (8 bytes) */
@@ -83,7 +85,7 @@ static void serial_process_irp_create(SERIAL_DEVICE* serial, IRP* irp)
 	Stream_Seek_UINT32(irp->input); /* CreateDisposition (4 bytes) */
 	Stream_Seek_UINT32(irp->input); /* CreateOptions (4 bytes) */

-	Stream_Read_UINT32(irp->input, PathLength);
+	Stream_Read_UINT32(irp->input, PathLength); /* PathLength (4 bytes) */

 	status = ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(irp->input),
 			PathLength / 2, &path, 0, NULL, NULL);
@@ -108,8 +110,8 @@ static void serial_process_irp_create(SERIAL_DEVICE* serial, IRP* irp)
 		DEBUG_SVC("%s(%d) created.", serial->path, FileId);
 	}

-	Stream_Write_UINT32(irp->output, FileId);
-	Stream_Write_UINT8(irp->output, 0);
+	Stream_Write_UINT32(irp->output, FileId); /* FileId (4 bytes) */
+	Stream_Write_UINT8(irp->output, 0); /* Information (1 byte) */

 	free(path);

@@ -118,9 +120,9 @@ static void serial_process_irp_create(SERIAL_DEVICE* serial, IRP* irp)

 static void serial_process_irp_close(SERIAL_DEVICE* serial, IRP* irp)
 {
-	SERIAL_TTY* tty;
+	SERIAL_TTY* tty = serial->tty;

-	tty = serial->tty;
+	Stream_Seek(irp->input, 32); /* Padding (32 bytes) */

 	if (!tty)
 	{
@@ -142,17 +144,14 @@ static void serial_process_irp_close(SERIAL_DEVICE* serial, IRP* irp)

 static void serial_process_irp_read(SERIAL_DEVICE* serial, IRP* irp)
 {
-	SERIAL_TTY* tty;
 	UINT32 Length;
 	UINT64 Offset;
 	BYTE* buffer = NULL;
+	SERIAL_TTY* tty = serial->tty;

-	Stream_Read_UINT32(irp->input, Length);
-	Stream_Read_UINT64(irp->input, Offset);
-
-	DEBUG_SVC("length %u offset %llu", Length, Offset);
-
-	tty = serial->tty;
+	Stream_Read_UINT32(irp->input, Length); /* Length (4 bytes) */
+	Stream_Read_UINT64(irp->input, Offset); /* Offset (8 bytes) */
+	Stream_Seek(irp->input, 20); /* Padding (20 bytes) */

 	if (!tty)
 	{
@@ -180,7 +179,7 @@ static void serial_process_irp_read(SERIAL_DEVICE* serial, IRP* irp)
 		}
 	}

-	Stream_Write_UINT32(irp->output, Length);
+	Stream_Write_UINT32(irp->output, Length); /* Length (4 bytes) */

 	if (Length > 0)
 	{
@@ -195,17 +194,13 @@ static void serial_process_irp_read(SERIAL_DEVICE* serial, IRP* irp)

 static void serial_process_irp_write(SERIAL_DEVICE* serial, IRP* irp)
 {
-	SERIAL_TTY* tty;
 	UINT32 Length;
 	UINT64 Offset;
+	SERIAL_TTY* tty = serial->tty;

-	Stream_Read_UINT32(irp->input, Length);
-	Stream_Read_UINT64(irp->input, Offset);
-	Stream_Seek(irp->input, 20); /* Padding */
-
-	DEBUG_SVC("length %u offset %llu", Length, Offset);
-
-	tty = serial->tty;
+	Stream_Read_UINT32(irp->input, Length); /* Length (4 bytes) */
+	Stream_Read_UINT64(irp->input, Offset); /* Offset (8 bytes) */
+	Stream_Seek(irp->input, 20); /* Padding (20 bytes) */

 	if (!tty)
 	{
@@ -226,26 +221,24 @@ static void serial_process_irp_write(SERIAL_DEVICE* serial, IRP* irp)
 		DEBUG_SVC("write %llu-%llu to %s(%d).", Offset, Offset + Length, serial->path, tty->id);
 	}

-	Stream_Write_UINT32(irp->output, Length);
-	Stream_Write_UINT8(irp->output, 0); /* Padding */
+	Stream_Write_UINT32(irp->output, Length); /* Length (4 bytes) */
+	Stream_Write_UINT8(irp->output, 0); /* Padding (1 byte) */

 	irp->Complete(irp);
 }

 static void serial_process_irp_device_control(SERIAL_DEVICE* serial, IRP* irp)
 {
-	SERIAL_TTY* tty;
 	UINT32 IoControlCode;
 	UINT32 InputBufferLength;
 	UINT32 OutputBufferLength;
 	UINT32 abortIo = SERIAL_ABORT_IO_NONE;
+	SERIAL_TTY* tty = serial->tty;

-	Stream_Read_UINT32(irp->input, InputBufferLength);
-	Stream_Read_UINT32(irp->input, OutputBufferLength);
-	Stream_Read_UINT32(irp->input, IoControlCode);
-	Stream_Seek(irp->input, 20); /* Padding */
-
-	tty = serial->tty;
+	Stream_Read_UINT32(irp->input, OutputBufferLength); /* OutputBufferLength (4 bytes) */
+	Stream_Read_UINT32(irp->input, InputBufferLength); /* InputBufferLength (4 bytes) */
+	Stream_Read_UINT32(irp->input, IoControlCode); /* IoControlCode (4 bytes) */
+	Stream_Seek(irp->input, 20); /* Padding (20 bytes) */

 	if (!tty)
 	{
@@ -264,7 +257,8 @@ static void serial_process_irp_device_control(SERIAL_DEVICE* serial, IRP* irp)

 static void serial_process_irp(SERIAL_DEVICE* serial, IRP* irp)
 {
-	DEBUG_SVC("MajorFunction %u", irp->MajorFunction);
+	WLog_Print(serial->log, WLOG_DEBUG, "IRP MajorFunction: 0x%04X MinorFunction: 0x%04X\n",
+			irp->MajorFunction, irp->MinorFunction);

 	switch (irp->MajorFunction)
 	{
@@ -333,7 +327,7 @@ static void serial_free(DEVICE* device)
 {
 	SERIAL_DEVICE* serial = (SERIAL_DEVICE*) device;

-	DEBUG_SVC("freeing device");
+	WLog_Print(serial->log, WLOG_DEBUG, "freeing");

 	MessageQueue_PostQuit(serial->IrpQueue, 0);
 	WaitForSingleObject(serial->thread, INFINITE);
@@ -391,6 +385,10 @@ int DeviceServiceEntry(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints)
 		serial->path = path;
 		serial->IrpQueue = MessageQueue_New(NULL);

+		WLog_Init();
+		serial->log = WLog_Get("com.freerdp.channel.serial.client");
+		WLog_Print(serial->log, WLOG_DEBUG, "initializing");
+
 		pEntryPoints->RegisterDevice(pEntryPoints->devman, (DEVICE*) serial);

 		serial->thread = CreateThread(NULL, 0, 
--- a/channels/serial/client/serial_tty.c
+++ b/channels/serial/client/serial_tty.c
@@ -73,24 +73,29 @@
 #define TIOCOUTQ FIONWRITE
 #endif

+/**
+ * Refer to ReactOS's ntddser.h (public domain) for constant definitions
+ */
+
 static UINT32 tty_write_data(SERIAL_TTY* tty, BYTE* data, int len);
 static void tty_set_termios(SERIAL_TTY* tty);
 static BOOL tty_get_termios(SERIAL_TTY* tty);
 static int tty_get_error_status();

-UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input, wStream* output, UINT32* abort_io)
+UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input, wStream* output, UINT32* abortIo)
 {
-	int purge_mask;
 	UINT32 result;
-	UINT32 modemstate;
 	BYTE immediate;
-	UINT32 ret = STATUS_SUCCESS;
-	UINT32 length = 0;
-	UINT32 pos;
+	int purge_mask;
+	UINT32 modemstate;
+	UINT32 begPos, endPos;
+	UINT32 OutputBufferLength;
+	UINT32 status = STATUS_SUCCESS;

 	DEBUG_SVC("in");

-	Stream_Seek(output, sizeof(UINT32));
+	Stream_Seek_UINT32(output); /* OutputBufferLength (4 bytes) */
+	begPos = (UINT32) Stream_GetPosition(output);

 	switch (IoControlCode)
 	{
@@ -101,7 +106,7 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,
 			break;

 		case IOCTL_SERIAL_GET_BAUD_RATE:
-			length = 4;
+			OutputBufferLength = 4;
 			Stream_Write_UINT32(output, tty->baud_rate);
 			DEBUG_SVC("SERIAL_GET_BAUD_RATE %d", tty->baud_rate);
 			break;
@@ -123,7 +128,7 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,

 		case IOCTL_SERIAL_GET_LINE_CONTROL:
 			DEBUG_SVC("SERIAL_GET_LINE_CONTROL");
-			length = 3;
+			OutputBufferLength = 3;
 			Stream_Write_UINT8(output, tty->stop_bits);
 			Stream_Write_UINT8(output, tty->parity);
 			Stream_Write_UINT8(output, tty->word_length);
@@ -137,13 +142,13 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,

 		case IOCTL_SERIAL_CONFIG_SIZE:
 			DEBUG_SVC("SERIAL_CONFIG_SIZE");
-			length = 4;
+			OutputBufferLength = 4;
 			Stream_Write_UINT32(output, 0);
 			break;

 		case IOCTL_SERIAL_GET_CHARS:
 			DEBUG_SVC("SERIAL_GET_CHARS");
-			length = 6;
+			OutputBufferLength = 6;
 			Stream_Write(output, tty->chars, 6);
 			break;

@@ -154,7 +159,7 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,
 			break;

 		case IOCTL_SERIAL_GET_HANDFLOW:
-			length = 16;
+			OutputBufferLength = 16;
 			tty_get_termios(tty);
 			Stream_Write_UINT32(output, tty->control);
 			Stream_Write_UINT32(output, tty->xonoff);
@@ -200,7 +205,7 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,
 				tty->read_interval_timeout,
 				tty->read_total_timeout_multiplier,
 				tty->read_total_timeout_constant);
-			length = 20;
+			OutputBufferLength = 20;
 			Stream_Write_UINT32(output, tty->read_interval_timeout);
 			Stream_Write_UINT32(output, tty->read_total_timeout_multiplier);
 			Stream_Write_UINT32(output, tty->read_total_timeout_constant);
@@ -210,7 +215,7 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,

 		case IOCTL_SERIAL_GET_WAIT_MASK:
 			DEBUG_SVC("SERIAL_GET_WAIT_MASK %X", tty->wait_mask);
-			length = 4;
+			OutputBufferLength = 4;
 			Stream_Write_UINT32(output, tty->wait_mask);
 			break;

@@ -269,12 +274,12 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,
 				modemstate |= SERIAL_MS_RTS;
 #endif
 			DEBUG_SVC("SERIAL_GET_MODEMSTATUS %X", modemstate);
-			length = 4;
+			OutputBufferLength = 4;
 			Stream_Write_UINT32(output, modemstate);
 			break;

 		case IOCTL_SERIAL_GET_COMMSTATUS:
-			length = 18;
+			OutputBufferLength = 18;
 			Stream_Write_UINT32(output, 0);	/* Errors */
 			Stream_Write_UINT32(output, 0);	/* Hold reasons */

@@ -316,21 +321,21 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,
 #endif

 			if (purge_mask & SERIAL_PURGE_TXABORT)
-				*abort_io |= SERIAL_ABORT_IO_WRITE;
+				*abortIo |= SERIAL_ABORT_IO_WRITE;
 			if (purge_mask & SERIAL_PURGE_RXABORT)
-				*abort_io |= SERIAL_ABORT_IO_READ;
+				*abortIo |= SERIAL_ABORT_IO_READ;
 			break;
 		case IOCTL_SERIAL_WAIT_ON_MASK:
 			DEBUG_SVC("SERIAL_WAIT_ON_MASK %X", tty->wait_mask);
 			tty->event_pending = 1;
-			length = 4;
+			OutputBufferLength = 4;
 			if (serial_tty_get_event(tty, &result))
 			{
 				DEBUG_SVC("WAIT end  event = %X", result);
 				Stream_Write_UINT32(output, result);
 				break;
 			}
-			ret = STATUS_PENDING;
+			status = STATUS_PENDING;
 			break;

 		case IOCTL_SERIAL_SET_BREAK_ON:
@@ -356,17 +361,27 @@ UINT32 serial_tty_control(SERIAL_TTY* tty, UINT32 IoControlCode, wStream* input,
 			break;

 		default:
-			DEBUG_SVC("NOT FOUND IoControlCode SERIAL IOCTL %d", IoControlCode);
+			DEBUG_SVC("NOT FOUND IoControlCode SERIAL IOCTL 0x%08X", IoControlCode);
 			return STATUS_INVALID_PARAMETER;
 	}

-	/* Write OutputBufferLength */
-	pos = Stream_GetPosition(output);
-	Stream_SetPosition(output, 16);
-	Stream_Write_UINT32(output, length);
-	Stream_SetPosition(output, pos);
+	endPos = (UINT32) Stream_GetPosition(output);
+	OutputBufferLength = endPos - begPos;

-	return ret;
+	if (OutputBufferLength < 1)
+	{
+		Stream_Write_UINT8(output, 0); /* Padding (1 byte) */
+		endPos = (UINT32) Stream_GetPosition(output);
+		OutputBufferLength = endPos - begPos;
+	}
+
+	Stream_SealLength(output);
+
+	Stream_SetPosition(output, 16);
+	Stream_Write_UINT32(output, OutputBufferLength); /* OutputBufferLength (4 bytes) */
+	Stream_SetPosition(output, endPos);
+
+	return status;
 }

 BOOL serial_tty_read(SERIAL_TTY* tty, BYTE* buffer, UINT32* Length)
@@ -472,7 +487,7 @@ void serial_tty_free(SERIAL_TTY* tty)
 {
 	DEBUG_SVC("in");

-	if(!tty)
+	if (!tty)
 		return;

 	if (tty->fd >= 0)
@@ -494,6 +509,9 @@ SERIAL_TTY* serial_tty_new(const char* path, UINT32 id)

 	tty = (SERIAL_TTY*) calloc(1, sizeof(SERIAL_TTY));

+	if (!tty)
+		return NULL;
+
 	tty->id = id;
 	tty->fd = open(path, O_RDWR | O_NOCTTY | O_NONBLOCK);

@@ -509,19 +527,17 @@ SERIAL_TTY* serial_tty_new(const char* path, UINT32 id)
 		DEBUG_SVC("tty fd %d successfully opened", tty->fd);
 	}

-	tty->ptermios = (struct termios*) malloc(sizeof(struct termios));
-	ZeroMemory(tty->ptermios, sizeof(struct termios));
+	tty->ptermios = (struct termios*) calloc(1, sizeof(struct termios));

-	if (tty->ptermios == NULL)
+	if (!tty->ptermios)
 	{
 		serial_tty_free(tty);
-		return NULL ;
+		return NULL;
 	}

-	tty->pold_termios = (struct termios*) malloc(sizeof(struct termios));
-	ZeroMemory(tty->pold_termios, sizeof(struct termios));
+	tty->pold_termios = (struct termios*) calloc(1, sizeof(struct termios));

-	if (tty->pold_termios == NULL)
+	if (!tty->pold_termios)
 	{
 		serial_tty_free(tty);
 		return NULL;
@@ -601,7 +617,6 @@ BOOL serial_tty_get_event(SERIAL_TTY* tty, UINT32* result)
 				*result |= SERIAL_EV_RLSD;
 				status = TRUE;
 			}
-
 		}

 		if ((bytes > 1) && (tty->wait_mask & SERIAL_EV_RXFLAG))
@@ -617,7 +632,6 @@ BOOL serial_tty_get_event(SERIAL_TTY* tty, UINT32* result)
 			*result |= SERIAL_EV_RXCHAR;
 			status = TRUE;
 		}
-
 	}
 	else
 	{
@@ -824,7 +838,7 @@ static BOOL tty_get_termios(SERIAL_TTY* tty)
 static void tty_set_termios(SERIAL_TTY* tty)
 {
 	speed_t speed;
-	struct termios *ptermios;
+	struct termios* ptermios;

 	DEBUG_SVC("in");
 	ptermios = tty->ptermios;