[ros-diffs] [reactos] 08/09: [CONSOLE.CPL] Fix problems detected by code analysis.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=38f6fd9df7f8d1f37d765…

commit 38f6fd9df7f8d1f37d7651683da7cf5af85d9f42
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Fri Jan 28 23:46:41 2022 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Sun Jan 30 01:02:10 2022 +0100

    [CONSOLE.CPL] Fix problems detected by code analysis.
    
    colors.c(129): warning C6287: Redundant code: the left and right sub-expressions are
identical.
    
    colors.c(159): warning C6001: Using uninitialized memory 'colorIndex'.
    colors.c(180): warning C6385: Reading invalid data from
'ConInfo->ColorTable': the readable size is '64' bytes, but
'259696' bytes may be read.
    (I thank ThFabba for the suggestion fix.)
    
    console.c(313): warning C6387: 'ConInfo->hWnd' could be '0': this
does not adhere to the specification for the function 'SendMessageW'.
    
    font.c(60): warning C6387: 'Preview->hFont' could be '0': this does
not adhere to the specification for the function 'GetFontCellSize'.
    font.c(230): warning C6001: Using uninitialized memory 'szFontSize'.
    font.c(230): warning C6054: String 'szFontSize' might not be zero-terminated.
    font.c(671): warning C6387: 'hFont' could be '0': this does not adhere
to the specification for the function 'GetFontCellSize'.
    
    layout.c(502): warning C6387: 'FontPreview.hFont' could be '0': this
does not adhere to the specification for the function 'SelectObject'.
---
 dll/cpl/console/colors.c  | 28 ++++++++++++++++------------
 dll/cpl/console/console.c | 18 ++++++++++++------
 dll/cpl/console/font.c    | 30 ++++++++++++++++++++++++------
 dll/cpl/console/layout.c  |  3 +++
 4 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/dll/cpl/console/colors.c b/dll/cpl/console/colors.c
index f6a8c40e947..fee811d89bc 100644
--- a/dll/cpl/console/colors.c
+++ b/dll/cpl/console/colors.c
@@ -126,14 +126,16 @@ ColorsProc(HWND hDlg,
         case WM_COMMAND:
         {
             /* NOTE: both BN_CLICKED and STN_CLICKED == 0 */
-            if (HIWORD(wParam) == BN_CLICKED || HIWORD(wParam) == STN_CLICKED)
+            if (HIWORD(wParam) == BN_CLICKED /* || HIWORD(wParam) == STN_CLICKED */)
             {
-                if (LOWORD(wParam) == IDC_RADIO_SCREEN_TEXT       ||
-                    LOWORD(wParam) == IDC_RADIO_SCREEN_BACKGROUND ||
-                    LOWORD(wParam) == IDC_RADIO_POPUP_TEXT        ||
-                    LOWORD(wParam) == IDC_RADIO_POPUP_BACKGROUND)
+                WORD ctrlIndex = LOWORD(wParam);
+
+                if (ctrlIndex == IDC_RADIO_SCREEN_TEXT       ||
+                    ctrlIndex == IDC_RADIO_SCREEN_BACKGROUND ||
+                    ctrlIndex == IDC_RADIO_POPUP_TEXT        ||
+                    ctrlIndex == IDC_RADIO_POPUP_BACKGROUND)
                 {
-                    switch (LOWORD(wParam))
+                    switch (ctrlIndex)
                     {
                     case IDC_RADIO_SCREEN_TEXT:
                         /* Get the colour of the screen foreground */
@@ -169,9 +171,9 @@ ColorsProc(HWND hDlg,
                     break;
                 }
                 else
-                if (IDC_STATIC_COLOR1 <= LOWORD(wParam) && LOWORD(wParam)
<= IDC_STATIC_COLOR16)
+                if (IDC_STATIC_COLOR1 <= ctrlIndex && ctrlIndex <=
IDC_STATIC_COLOR16)
                 {
-                    colorIndex = LOWORD(wParam) - IDC_STATIC_COLOR1;
+                    colorIndex = ctrlIndex - IDC_STATIC_COLOR1;
 
                     /* If the same static control was re-clicked, don't take it into
account */
                     if (colorIndex == ActiveStaticControl)
@@ -213,9 +215,11 @@ ColorsProc(HWND hDlg,
             }
             else if (HIWORD(wParam) == EN_KILLFOCUS)
             {
-                if (LOWORD(wParam) == IDC_EDIT_COLOR_RED   ||
-                    LOWORD(wParam) == IDC_EDIT_COLOR_GREEN ||
-                    LOWORD(wParam) == IDC_EDIT_COLOR_BLUE)
+                WORD ctrlIndex = LOWORD(wParam);
+
+                if (ctrlIndex == IDC_EDIT_COLOR_RED   ||
+                    ctrlIndex == IDC_EDIT_COLOR_GREEN ||
+                    ctrlIndex == IDC_EDIT_COLOR_BLUE)
                 {
                     DWORD value;
 
@@ -224,7 +228,7 @@ ColorsProc(HWND hDlg,
                     color = ConInfo->ColorTable[colorIndex];
 
                     /* Modify the colour component */
-                    switch (LOWORD(wParam))
+                    switch (ctrlIndex)
                     {
                     case IDC_EDIT_COLOR_RED:
                         value = GetDlgItemInt(hDlg, IDC_EDIT_COLOR_RED, NULL, FALSE);
diff --git a/dll/cpl/console/console.c b/dll/cpl/console/console.c
index 09c204c630c..65640bc9282 100644
--- a/dll/cpl/console/console.c
+++ b/dll/cpl/console/console.c
@@ -112,7 +112,7 @@ ApplyConsoleInfo(HWND hwndDlg)
         SetConsoleInfo  = (res != IDCANCEL);
         SaveConsoleInfo = (res == IDC_RADIO_APPLY_ALL);
 
-        if (SetConsoleInfo == FALSE)
+        if (!SetConsoleInfo)
         {
             /* Don't destroy when the user presses cancel */
             SetWindowLongPtr(hwndDlg, DWLP_MSGRESULT, PSNRET_INVALID_NOCHANGEPAGE);
@@ -275,7 +275,7 @@ InitApplet(HANDLE hSectionOrWnd)
     ResetFontPreview(&FontPreview);
     ClearTTFontCache();
 
-    /* Save the console settings */
+    /* Apply the console settings if necessary */
     if (SetConsoleInfo)
     {
         HANDLE hSection;
@@ -303,19 +303,25 @@ InitApplet(HANDLE hSectionOrWnd)
             goto Quit;
         }
 
-        /* Copy the console information into the section */
+        /* Copy the console information into the section and unmap it */
         RtlCopyMemory(pSharedInfo, ConInfo, ConInfo->cbSize);
-
-        /* Unmap it */
         UnmapViewOfFile(pSharedInfo);
 
-        /* Signal to CONSRV that it can apply the new configuration */
+        /*
+         * Signal to CONSRV that it can apply the new settings.
+         *
+         * NOTE: SetConsoleInfo set to TRUE by ApplyConsoleInfo()
+         * *only* when ConInfo->hWnd != NULL and the user did not
+         * press IDCANCEL in the confirmation dialog.
+         */
+        ASSERT(ConInfo->hWnd);
         SendMessageW(ConInfo->hWnd, WM_SETCONSOLEINFO, (WPARAM)hSection, 0);
 
         /* Close the section and return */
         CloseHandle(hSection);
     }
 
+    /* Save the console settings */
     if (SaveConsoleInfo)
     {
         /* Default settings saved when ConInfo->hWnd == NULL */
diff --git a/dll/cpl/console/font.c b/dll/cpl/console/font.c
index 0909417dfee..4cb5f4c61d3 100644
--- a/dll/cpl/console/font.c
+++ b/dll/cpl/console/font.c
@@ -53,11 +53,16 @@ RefreshFontPreview(
     IN FONT_PREVIEW* Preview,
     IN PCONSOLE_STATE_INFO pConInfo)
 {
-    if (Preview->hFont) DeleteObject(Preview->hFont);
-    Preview->hFont = CreateConsoleFont(pConInfo);
-    if (Preview->hFont == NULL)
+    HFONT hFont = CreateConsoleFont(pConInfo);
+    if (!hFont)
+    {
         DPRINT1("RefreshFontPreview: CreateConsoleFont() failed\n");
-    GetFontCellSize(NULL, Preview->hFont, &Preview->CharHeight,
&Preview->CharWidth);
+        return;
+    }
+
+    if (Preview->hFont) DeleteObject(Preview->hFont);
+    Preview->hFont = hFont;
+    GetFontCellSize(NULL, hFont, &Preview->CharHeight,
&Preview->CharWidth);
 }
 
 VOID
@@ -223,8 +228,17 @@ FontSizeList_GetSelectedFontSize(
              * See:
https://support.microsoft.com/en-us/help/66365/how-to-process-a-cbn-selchan…
              * for more details.
              */
+            INT Length;
+
             nSel = (INT)SendMessageW(SizeList->hWndTTSizeList, CB_GETCURSEL, 0, 0);
-            SendMessageW(SizeList->hWndTTSizeList, CB_GETLBTEXT, nSel,
(LPARAM)szFontSize);
+            if (nSel == CB_ERR) return 0;
+
+            Length = (INT)SendMessageW(SizeList->hWndTTSizeList, CB_GETLBTEXTLEN,
nSel, 0);
+            ASSERT((Length != LB_ERR) && (Length < ARRAYSIZE(szFontSize)));
+
+            Length = (INT)SendMessageW(SizeList->hWndTTSizeList, CB_GETLBTEXT, nSel,
(LPARAM)szFontSize);
+            ASSERT((Length != LB_ERR) && (Length < ARRAYSIZE(szFontSize)));
+            szFontSize[Length] = L'\0';
 
             /* Validate the font size */
             FontSize = wcstoul(szFontSize, &pszNext, 10);
@@ -558,6 +572,7 @@ FontTypeChange(
     if (FaceName == NULL) return FALSE;
 
     Length = (INT)SendMessageW(hFontList, LB_GETTEXT, nSel, (LPARAM)FaceName);
+    ASSERT(Length != LB_ERR);
     FaceName[Length] = L'\0';
 
     StringCchCopyW(pConInfo->FaceName, ARRAYSIZE(pConInfo->FaceName), FaceName);
@@ -664,8 +679,11 @@ FontSizeChange(
     CharWidth  = (UINT)(SizeList->UseRasterOrTTList ? LOWORD(FontSize) : 0);
 
     hFont = CreateConsoleFont2((LONG)CharHeight, (LONG)CharWidth, pConInfo);
-    if (hFont == NULL)
+    if (!hFont)
+    {
         DPRINT1("FontSizeChange: CreateConsoleFont2() failed\n");
+        return FALSE;
+    }
 
     /* Retrieve the real character size in pixels */
     GetFontCellSize(NULL, hFont, &CharHeight, &CharWidth);
diff --git a/dll/cpl/console/layout.c b/dll/cpl/console/layout.c
index 8430b3de218..e275b518b9f 100644
--- a/dll/cpl/console/layout.c
+++ b/dll/cpl/console/layout.c
@@ -497,6 +497,9 @@ PaintText(
     /* Refresh the font preview, getting a new font if necessary */
     if (FontPreview.hFont == NULL)
         RefreshFontPreview(&FontPreview, pConInfo);
+    /* Recheck hFont since RefreshFontPreview() may not have updated it */
+    if (FontPreview.hFont == NULL)
+        return;
 
     /* Draw the preview text using the current font */
     hOldFont = SelectObject(drawItem->hDC, FontPreview.hFont);