[hbelusca] 72000: [MMSYS]: Improve AddSoundProfile, by Victor Martinez Calvo (with 1 minor modification by myself): ◾ Don't return TRUE when CB_SETITEMDATA fails. ◾ Avoid pScheme NULL dereference i...

Author: hbelusca
Date: Mon Jul 25 20:24:30 2016
New Revision: 72000

URL: http://svn.reactos.org/svn/reactos?rev=72000&view=rev
Log:
[MMSYS]: Improve AddSoundProfile, by Victor Martinez Calvo (with 1 minor modification by myself):
◾ Don't return TRUE when CB_SETITEMDATA fails.
◾ Avoid pScheme NULL dereference if allocation fails. CID 1223154
◾ Avoid overflowing the destiny buffer by using StringCchCopy() CID 510953
and:
1) Make it more readable by returning as soon as possible when an error happens.
2) Make it more readable by reducing the nested ifs-checks.
3) Remove the added string in the combobox if the sound scheme buffer mem allocation fails (pt.3 modified).
CORE-11603 #resolve #comment Thanks!

Modified:
    trunk/reactos/dll/cpl/mmsys/sounds.c

Modified: trunk/reactos/dll/cpl/mmsys/sounds.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/cpl/mmsys/sounds.c?rev=...
==============================================================================
--- trunk/reactos/dll/cpl/mmsys/sounds.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/cpl/mmsys/sounds.c	[iso-8859-1] Mon Jul 25 20:24:30 2016
@@ -5,11 +5,14 @@
  * PROGRAMMER:      Thomas Weidenmueller [e-mail]
  *                  Johannes Anderwald [e-mail]
  *                  Dmitry Chapyshev [e-mail]
+ *                  Victor Martinez Calvo [e-mail]
  */

#include "mmsys.h"

#include <commdlg.h>
+#include <strsafe.h>
+
 #include <debug.h>

struct __APP_MAP__;
@@ -291,6 +294,8 @@
     HKEY hSubKey;
     TCHAR szValue[MAX_PATH];
     DWORD dwValue, dwResult;
+    LRESULT lResult;
+    PSOUND_SCHEME_CONTEXT pScheme;

if (RegOpenKeyEx(hKey,
                      szSubKey,
@@ -309,27 +314,35 @@
                                (LPBYTE)szValue,
                                &dwValue);
     RegCloseKey(hSubKey);
-    if (dwResult == ERROR_SUCCESS)
-    {
-        LRESULT lResult = SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_ADDSTRING, (WPARAM)0, (LPARAM)szValue);
-        if (lResult != CB_ERR)
-        {
-            PSOUND_SCHEME_CONTEXT pScheme = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(SOUND_SCHEME_CONTEXT));
-            if (pScheme != NULL)
-            {
-                _tcscpy(pScheme->szDesc, szValue);
-                _tcscpy(pScheme->szName, szSubKey);
-            }
-
-            SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_SETITEMDATA, (WPARAM)lResult, (LPARAM)pScheme);
-            if (SetDefault)
-            {
-                SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_SETCURSEL, (WPARAM)lResult, (LPARAM)0);
-            }
-        }
-        return TRUE;
-    }
-    return FALSE;
+
+    if (dwResult != ERROR_SUCCESS)
+        return FALSE;
+
+    /* Try to add the new profile */
+    lResult = SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_ADDSTRING, (WPARAM)0, (LPARAM)szValue);
+    if (lResult == CB_ERR)
+        return FALSE;
+
+    /* Allocate the profile scheme buffer */
+    pScheme = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(SOUND_SCHEME_CONTEXT));
+    if (pScheme == NULL)
+    {
+        /* We failed to allocate the buffer, no need to keep a dangling string in the combobox */
+        SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_DELETESTRING, (WPARAM)lResult, (LPARAM)0);
+        return FALSE;
+    }
+
+    StringCchCopy(pScheme->szDesc, MAX_PATH, szValue);
+    StringCchCopy(pScheme->szName, MAX_PATH, szSubKey);
+
+    /* Associate the value with the item in the combobox */
+    SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_SETITEMDATA, (WPARAM)lResult, (LPARAM)pScheme);
+
+    /* Optionally, select the profile */ 
+    if (SetDefault)
+        SendDlgItemMessage(hwndDlg, IDC_SOUND_SCHEME, CB_SETCURSEL, (WPARAM)lResult, (LPARAM)0);
+
+    return TRUE;
 }