From c4d60756dd48a07d30a578959100dfd2e37d0b72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Thu, 19 Mar 2026 22:45:40 +0100 Subject: [PATCH] [FREELDR][KDCOM][KDGDB][NTOS:KD] Some code "nits" - Don't hardcode constant string lengths. - Use `_strnicmp()` -- Ideally we shouldn't have to unconditionally upcase the global kernel command-line string to perform substrings comparisons. - Cast `atol()` returned value to `ULONG`. --- boot/freeldr/freeldr/lib/debug.c | 22 ++++++++++++---------- drivers/base/kdcom/kdcom.c | 25 +++++++++++-------------- drivers/base/kdgdb/kdcom.c | 28 +++++++++++++--------------- ntoskrnl/kd/kdmain.c | 18 +++++++++--------- 4 files changed, 45 insertions(+), 48 deletions(-) diff --git a/boot/freeldr/freeldr/lib/debug.c b/boot/freeldr/freeldr/lib/debug.c index e6d2cf6538b..1654dd4bdce 100644 --- a/boot/freeldr/freeldr/lib/debug.c +++ b/boot/freeldr/freeldr/lib/debug.c @@ -58,6 +58,8 @@ VOID DebugInit( _In_ PCSTR DebugString) { +#define CONST_STR_LEN(x) (sizeof(x)/sizeof(x[0]) - 1) + static BOOLEAN Initialized = FALSE; PSTR CommandLine, PortString, BaudString; ULONG Value; @@ -114,30 +116,30 @@ DebugInit( while (PortString) { /* Move past the actual string */ - PortString += strlen("DEBUGPORT"); + PortString += CONST_STR_LEN("DEBUGPORT"); /* Now get past any spaces and skip the equal sign */ while (*PortString == ' ') PortString++; PortString++; /* Check for possible ports and set the port to use */ - if (strncmp(PortString, "SCREEN", 6) == 0) + if (_strnicmp(PortString, "SCREEN", CONST_STR_LEN("SCREEN")) == 0) { - PortString += 6; + PortString += CONST_STR_LEN("SCREEN"); DebugPort |= SCREEN; } - else if (strncmp(PortString, "BOCHS", 5) == 0) + else if (_strnicmp(PortString, "BOCHS", CONST_STR_LEN("BOCHS")) == 0) { - PortString += 5; + PortString += CONST_STR_LEN("BOCHS"); DebugPort |= BOCHS; } - else if (strncmp(PortString, "COM", 3) == 0) + else if (_strnicmp(PortString, "COM", CONST_STR_LEN("COM")) == 0) { - PortString += 3; + PortString += CONST_STR_LEN("COM"); DebugPort |= RS232; /* Set the port to use */ - Value = atol(PortString); + Value = (ULONG)atol(PortString); if (Value) ComPort = Value; } @@ -148,14 +150,14 @@ DebugInit( if (BaudString) { /* Move past the actual string and any spaces */ - BaudString += strlen("BAUDRATE"); + BaudString += CONST_STR_LEN("BAUDRATE"); while (*BaudString == ' ') BaudString++; /* Make sure we have a rate */ if (*BaudString) { /* Read and set it */ - Value = atol(BaudString + 1); + Value = (ULONG)atol(BaudString + 1); if (Value) BaudRate = Value; } } diff --git a/drivers/base/kdcom/kdcom.c b/drivers/base/kdcom/kdcom.c index bc8c07bde2d..7b4716a5677 100644 --- a/drivers/base/kdcom/kdcom.c +++ b/drivers/base/kdcom/kdcom.c @@ -9,7 +9,6 @@ #include "kddll.h" #include -#include #include #include @@ -125,6 +124,8 @@ NTSTATUS NTAPI KdDebuggerInitialize0(IN PLOADER_PARAMETER_BLOCK LoaderBlock OPTIONAL) { +#define CONST_STR_LEN(x) (sizeof(x)/sizeof(x[0]) - 1) + ULONG ComPortNumber = DEFAULT_DEBUG_PORT; ULONG ComPortBaudRate = DEFAULT_DEBUG_BAUD_RATE; @@ -148,42 +149,38 @@ KdDebuggerInitialize0(IN PLOADER_PARAMETER_BLOCK LoaderBlock OPTIONAL) if (PortString) { /* Move past the actual string */ - PortString += strlen("DEBUGPORT"); + PortString += CONST_STR_LEN("DEBUGPORT"); /* Now get past any spaces and skip the equal sign */ while (*PortString == ' ') PortString++; PortString++; /* Do we have a serial port? */ - if (strncmp(PortString, "COM", 3) != 0) - { + if (_strnicmp(PortString, "COM", CONST_STR_LEN("COM")) != 0) return STATUS_INVALID_PARAMETER; - } /* Check for a valid serial port */ - PortString += 3; - Value = atol(PortString); - if (Value >= sizeof(BaseArray) / sizeof(BaseArray[0])) - { + PortString += CONST_STR_LEN("COM"); + Value = (ULONG)atol(PortString); + if (Value > MAX_COM_PORTS) return STATUS_INVALID_PARAMETER; - } - + // if (Value > 0 && Value <= MAX_COM_PORTS) /* Set the port to use */ ComPortNumber = Value; - } + } /* Check if we got a baud rate */ if (BaudString) { /* Move past the actual string and any spaces */ - BaudString += strlen("BAUDRATE"); + BaudString += CONST_STR_LEN("BAUDRATE"); while (*BaudString == ' ') BaudString++; /* Make sure we have a rate */ if (*BaudString) { /* Read and set it */ - Value = atol(BaudString + 1); + Value = (ULONG)atol(BaudString + 1); if (Value) ComPortBaudRate = Value; } } diff --git a/drivers/base/kdgdb/kdcom.c b/drivers/base/kdgdb/kdcom.c index 683eb379684..310438e9cfa 100644 --- a/drivers/base/kdgdb/kdcom.c +++ b/drivers/base/kdgdb/kdcom.c @@ -119,6 +119,8 @@ NTSTATUS NTAPI KdDebuggerInitialize0(IN PLOADER_PARAMETER_BLOCK LoaderBlock OPTIONAL) { +#define CONST_STR_LEN(x) (sizeof(x)/sizeof(x[0]) - 1) + ULONG ComPortNumber = DEFAULT_DEBUG_PORT; ULONG ComPortBaudRate = DEFAULT_DEBUG_BAUD_RATE; @@ -142,42 +144,38 @@ KdDebuggerInitialize0(IN PLOADER_PARAMETER_BLOCK LoaderBlock OPTIONAL) if (PortString) { /* Move past the actual string */ - PortString += strlen("DEBUGPORT"); + PortString += CONST_STR_LEN("DEBUGPORT"); /* Now get past any spaces and skip the equal sign */ while (*PortString == ' ') PortString++; PortString++; /* Do we have a serial port? */ - if (strncmp(PortString, "COM", 3) != 0) - { + if (_strnicmp(PortString, "COM", CONST_STR_LEN("COM")) != 0) return STATUS_INVALID_PARAMETER; - } /* Check for a valid serial port */ - PortString += 3; - Value = atol(PortString); - if (Value >= sizeof(BaseArray) / sizeof(BaseArray[0])) - { + PortString += CONST_STR_LEN("COM"); + Value = (ULONG)atol(PortString); + if (Value > MAX_COM_PORTS) return STATUS_INVALID_PARAMETER; - } - + // if (Value > 0 && Value <= MAX_COM_PORTS) /* Set the port to use */ ComPortNumber = Value; - } + } /* Check if we got a baud rate */ if (BaudString) { /* Move past the actual string and any spaces */ - BaudString += strlen("BAUDRATE"); + BaudString += CONST_STR_LEN("BAUDRATE"); while (*BaudString == ' ') BaudString++; /* Make sure we have a rate */ if (*BaudString) { /* Read and set it */ - Value = atol(BaudString + 1); + Value = (ULONG)atol(BaudString + 1); if (Value) ComPortBaudRate = Value; } } @@ -226,7 +224,7 @@ KdDebuggerInitialize1(IN PLOADER_PARAMETER_BLOCK LoaderBlock OPTIONAL) VOID NTAPI -KdpSendByte(_In_ UCHAR Byte) +KdpSendByte(IN UCHAR Byte) { /* Send the byte */ CpPutByte(&KdComPort, Byte); @@ -249,7 +247,7 @@ KdpPollByte(OUT PUCHAR OutByte) KDSTATUS NTAPI -KdpReceiveByte(_Out_ PUCHAR OutByte) +KdpReceiveByte(OUT PUCHAR OutByte) { USHORT CpStatus; diff --git a/ntoskrnl/kd/kdmain.c b/ntoskrnl/kd/kdmain.c index 9e9c71b091c..3c3c2ffe097 100644 --- a/ntoskrnl/kd/kdmain.c +++ b/ntoskrnl/kd/kdmain.c @@ -21,12 +21,12 @@ /* PUBLIC FUNCTIONS *********************************************************/ +#define CONST_STR_LEN(x) (sizeof(x)/sizeof(x[0]) - 1) + static VOID KdpGetTerminalSettings( _In_ PCSTR p1) { -#define CONST_STR_LEN(x) (sizeof(x)/sizeof(x[0]) - 1) - while (p1 && *p1) { /* Skip leading whitespace */ @@ -57,17 +57,17 @@ KdpGetDebugMode( ULONG Value; /* Check for Screen Debugging */ - if (!_strnicmp(p2, "SCREEN", 6)) + if (!_strnicmp(p2, "SCREEN", CONST_STR_LEN("SCREEN"))) { /* Enable it */ - p2 += 6; + p2 += CONST_STR_LEN("SCREEN"); KdpDebugMode.Screen = TRUE; } /* Check for Serial Debugging */ - else if (!_strnicmp(p2, "COM", 3)) + else if (!_strnicmp(p2, "COM", CONST_STR_LEN("COM"))) { /* Check for a valid serial port */ - p2 += 3; + p2 += CONST_STR_LEN("COM"); if (*p2 != ':') { Value = (ULONG)atol(p2); @@ -92,10 +92,10 @@ KdpGetDebugMode( } } /* Check for Debug Log Debugging */ - else if (!_strnicmp(p2, "FILE", 4)) + else if (!_strnicmp(p2, "FILE", CONST_STR_LEN("FILE"))) { /* Enable it */ - p2 += 4; + p2 += CONST_STR_LEN("FILE"); KdpDebugMode.File = TRUE; if (*p2 == ':') { @@ -140,7 +140,7 @@ KdDebuggerInitialize0( while (Port) { /* Move past the actual string */ - Port += sizeof("DEBUGPORT") - 1; + Port += CONST_STR_LEN("DEBUGPORT"); /* Now get past any spaces and skip the equal sign */ while (*Port == ' ') Port++;