[ros-diffs] [reactos] 01/01: [SERVMAN]

[Ged Murphy]

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

commit e36b89addba874fc9aa898a42e894ef37465cd30
Author: Ged Murphy <gedmurphy(a)reactos.org>
AuthorDate: Tue Dec 5 22:13:02 2017 +0000

    [SERVMAN]
    
    - Avoid a potential race whereby the current service selection can change before the
propsheet thread starts up
    - Cleanup the depends data, it doesn't need to be passed around the propsheet
---
 .../mscutils/servman/dependencies_tv1.c            | 29 ++++-----
 .../mscutils/servman/dependencies_tv2.c            | 24 +++----
 base/applications/mscutils/servman/precomp.h       | 17 +++--
 base/applications/mscutils/servman/propsheet.c     | 75 +++++++++++-----------
 .../mscutils/servman/propsheet_depends.c           | 54 ++++++++--------
 .../mscutils/servman/propsheet_recovery.c          |  3 +-
 6 files changed, 107 insertions(+), 95 deletions(-)

diff --git a/base/applications/mscutils/servman/dependencies_tv1.c
b/base/applications/mscutils/servman/dependencies_tv1.c
index 5e764e51db..27d376ea54 100644
--- a/base/applications/mscutils/servman/dependencies_tv1.c
+++ b/base/applications/mscutils/servman/dependencies_tv1.c
@@ -10,8 +10,7 @@
 #include "precomp.h"
 
 LPWSTR
-TV1_GetDependants(PSERVICEPROPSHEET pDlgInfo,
-                  SC_HANDLE hService)
+TV1_GetDependants(SC_HANDLE hService)
 {
     LPQUERY_SERVICE_CONFIG lpServiceConfig;
     LPWSTR lpStr = NULL;
@@ -80,7 +79,7 @@ TV1_GetDependants(PSERVICEPROPSHEET pDlgInfo,
 }
 
 VOID
-TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
+TV1_AddDependantsToTree(PDEPENDDATA pDependData,
                         HTREEITEM hParent,
                         LPWSTR lpServiceName)
 {
@@ -103,7 +102,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
         if (hService)
         {
             /* Get a list of service dependents */
-            lpDependants = TV1_GetDependants(pDlgInfo, hService);
+            lpDependants = TV1_GetDependants(hService);
             if (lpDependants)
             {
                 lpStr = lpDependants;
@@ -127,7 +126,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
                         }
 
                         /* Add it */
-                        AddItemToTreeView(pDlgInfo->hDependsTreeView1,
+                        AddItemToTreeView(pDependData->hDependsTreeView1,
                                           hParent,
                                           lpServiceConfig->lpDisplayName,
                                           lpStr,
@@ -156,7 +155,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
                     /* Load the 'No dependencies' string */
                     AllocAndLoadString(&lpNoDepends, hInstance, IDS_NO_DEPENDS);
 
-                    AddItemToTreeView(pDlgInfo->hDependsTreeView1,
+                    AddItemToTreeView(pDependData->hDependsTreeView1,
                                       NULL,
                                       lpNoDepends,
                                       NULL,
@@ -166,7 +165,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
                     LocalFree(lpNoDepends);
 
                     /* Disable the window */
-                    EnableWindow(pDlgInfo->hDependsTreeView1, FALSE);
+                    EnableWindow(pDependData->hDependsTreeView1, FALSE);
                 }
             }
 
@@ -178,25 +177,25 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
 }
 
 BOOL
-TV1_Initialize(PSERVICEPROPSHEET pDlgInfo,
+TV1_Initialize(PDEPENDDATA pDependData,
                LPWSTR lpServiceName)
 {
     BOOL bRet = FALSE;
 
     /* Associate the imagelist with TV1 */
-    pDlgInfo->hDependsTreeView1 = GetDlgItem(pDlgInfo->hDependsWnd,
IDC_DEPEND_TREE1);
-    if (!pDlgInfo->hDependsTreeView1)
+    pDependData->hDependsTreeView1 = GetDlgItem(pDependData->hDependsWnd,
IDC_DEPEND_TREE1);
+    if (!pDependData->hDependsTreeView1)
     {
-        ImageList_Destroy(pDlgInfo->hDependsImageList);
-        pDlgInfo->hDependsImageList = NULL;
+        ImageList_Destroy(pDependData->hDependsImageList);
+        pDependData->hDependsImageList = NULL;
         return FALSE;
     }
-    (void)TreeView_SetImageList(pDlgInfo->hDependsTreeView1,
-                                pDlgInfo->hDependsImageList,
+    (void)TreeView_SetImageList(pDependData->hDependsTreeView1,
+                                pDependData->hDependsImageList,
                                 TVSIL_NORMAL);
 
     /* Set the first items in the control */
-    TV1_AddDependantsToTree(pDlgInfo, NULL, lpServiceName);
+    TV1_AddDependantsToTree(pDependData, NULL, lpServiceName);
 
     return bRet;
 }
diff --git a/base/applications/mscutils/servman/dependencies_tv2.c
b/base/applications/mscutils/servman/dependencies_tv2.c
index a0f37b07c1..6f23197c0a 100644
--- a/base/applications/mscutils/servman/dependencies_tv2.c
+++ b/base/applications/mscutils/servman/dependencies_tv2.c
@@ -118,7 +118,7 @@ TV2_GetDependants(LPWSTR lpServiceName,
 }
 
 VOID
-TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
+TV2_AddDependantsToTree(PDEPENDDATA pDependData,
                         HTREEITEM hParent,
                         LPWSTR lpServiceName)
 {
@@ -138,7 +138,7 @@ TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
             bHasChildren = TV2_HasDependantServices(lpServiceStatus[i].lpServiceName);
 
             /* Add it */
-            AddItemToTreeView(pDlgInfo->hDependsTreeView2,
+            AddItemToTreeView(pDependData->hDependsTreeView2,
                               hParent,
                               lpServiceStatus[i].lpDisplayName,
                               lpServiceStatus[i].lpServiceName,
@@ -158,7 +158,7 @@ TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
             /* Load the 'No dependencies' string */
             AllocAndLoadString(&lpNoDepends, hInstance, IDS_NO_DEPENDS);
 
-            AddItemToTreeView(pDlgInfo->hDependsTreeView2,
+            AddItemToTreeView(pDependData->hDependsTreeView2,
                               NULL,
                               lpNoDepends,
                               NULL,
@@ -168,31 +168,31 @@ TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo,
             LocalFree(lpNoDepends);
 
             /* Disable the window */
-            EnableWindow(pDlgInfo->hDependsTreeView2, FALSE);
+            EnableWindow(pDependData->hDependsTreeView2, FALSE);
         }
     }
 }
 
 BOOL
-TV2_Initialize(PSERVICEPROPSHEET pDlgInfo,
+TV2_Initialize(PDEPENDDATA pDependData,
                LPWSTR lpServiceName)
 {
     BOOL bRet = FALSE;
 
     /* Associate the imagelist with TV2 */
-    pDlgInfo->hDependsTreeView2 = GetDlgItem(pDlgInfo->hDependsWnd,
IDC_DEPEND_TREE2);
-    if (!pDlgInfo->hDependsTreeView2)
+    pDependData->hDependsTreeView2 = GetDlgItem(pDependData->hDependsWnd,
IDC_DEPEND_TREE2);
+    if (!pDependData->hDependsTreeView2)
     {
-        ImageList_Destroy(pDlgInfo->hDependsImageList);
-        pDlgInfo->hDependsImageList = NULL;
+        ImageList_Destroy(pDependData->hDependsImageList);
+        pDependData->hDependsImageList = NULL;
         return FALSE;
     }
-    (void)TreeView_SetImageList(pDlgInfo->hDependsTreeView2,
-                                pDlgInfo->hDependsImageList,
+    (void)TreeView_SetImageList(pDependData->hDependsTreeView2,
+                                pDependData->hDependsImageList,
                                 TVSIL_NORMAL);
 
     /* Set the first items in the control */
-    TV2_AddDependantsToTree(pDlgInfo, NULL, lpServiceName);
+    TV2_AddDependantsToTree(pDependData, NULL, lpServiceName);
 
     return bRet;
 }
diff --git a/base/applications/mscutils/servman/precomp.h
b/base/applications/mscutils/servman/precomp.h
index 051634dc95..5ec27109f6 100644
--- a/base/applications/mscutils/servman/precomp.h
+++ b/base/applications/mscutils/servman/precomp.h
@@ -128,11 +128,18 @@ typedef struct _SERVICEPROPSHEET
 {
     PMAIN_WND_INFO Info;
     ENUM_SERVICE_STATUS_PROCESS *pService;
+
+} SERVICEPROPSHEET, *PSERVICEPROPSHEET;
+
+typedef struct _DEPENDDATA
+{
+    PSERVICEPROPSHEET pDlgInfo;
     HIMAGELIST hDependsImageList;
     HWND hDependsWnd;
     HWND hDependsTreeView1;
     HWND hDependsTreeView2;
-} SERVICEPROPSHEET, *PSERVICEPROPSHEET;
+
+} DEPENDDATA, *PDEPENDDATA;
 
 
 HTREEITEM AddItemToTreeView(HWND hTreeView, HTREEITEM hRoot, LPWSTR lpDisplayName, LPWSTR
lpServiceName, ULONG serviceType, BOOL bHasChildren);
@@ -147,12 +154,12 @@ LPWSTR DisplayName,
 LPWSTR ServiceList);
 
 /* tv1_dependencies */
-BOOL TV1_Initialize(PSERVICEPROPSHEET pDlgInfo, LPWSTR lpServiceName);
-VOID TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, HTREEITEM hParent, LPWSTR
lpServiceName);
+BOOL TV1_Initialize(PDEPENDDATA pDependData, LPWSTR lpServiceName);
+VOID TV1_AddDependantsToTree(PDEPENDDATA pDependData, HTREEITEM hParent, LPWSTR
lpServiceName);
 
 /* tv2_dependencies */
-BOOL TV2_Initialize(PSERVICEPROPSHEET pDlgInfo, LPWSTR lpServiceName);
-VOID TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, HTREEITEM hParent, LPWSTR
lpServiceName);
+BOOL TV2_Initialize(PDEPENDDATA pDependData, LPWSTR lpServiceName);
+VOID TV2_AddDependantsToTree(PDEPENDDATA pDependData, HTREEITEM hParent, LPWSTR
lpServiceName);
 BOOL TV2_HasDependantServices(LPWSTR lpServiceName);
 LPENUM_SERVICE_STATUS TV2_GetDependants(LPWSTR lpServiceName, LPDWORD lpdwCount);
 
diff --git a/base/applications/mscutils/servman/propsheet.c
b/base/applications/mscutils/servman/propsheet.c
index 9557420ebb..bd0ec24510 100644
--- a/base/applications/mscutils/servman/propsheet.c
+++ b/base/applications/mscutils/servman/propsheet.c
@@ -3,7 +3,7 @@
  * LICENSE:     GPL - See COPYING in the top level directory
  * FILE:        base/applications/mscutils/servman/propsheet.c
  * PURPOSE:     Property dialog box message handler
- * COPYRIGHT:   Copyright 2006-2007 Ged Murphy <gedmurphy(a)reactos.org>
+ * COPYRIGHT:   Copyright 2006-2017 Ged Murphy <gedmurphy(a)reactos.org>
  *
  */
 
@@ -29,8 +29,20 @@ InitPropSheetPage(PROPSHEETPAGE *psp,
 VOID
 OpenPropSheet(PMAIN_WND_INFO Info)
 {
+    PSERVICEPROPSHEET pServicePropSheet;
     HANDLE hThread;
-    hThread = (HANDLE)_beginthreadex(NULL, 0, &PropSheetThread, Info, 0, NULL);
+
+    pServicePropSheet = HeapAlloc(ProcessHeap,
+                                  0,
+                                  sizeof(*pServicePropSheet));
+    if (!pServicePropSheet) return;
+
+    /* Set the current service in this calling thread to avoid
+     * it being updated before the thread is up */
+    pServicePropSheet->pService = Info->pCurrentService;
+    pServicePropSheet->Info = Info;
+
+    hThread = (HANDLE)_beginthreadex(NULL, 0, &PropSheetThread, pServicePropSheet, 0,
NULL);
     if (hThread)
     {
         CloseHandle(hThread);
@@ -40,65 +52,54 @@ OpenPropSheet(PMAIN_WND_INFO Info)
 
 unsigned int __stdcall PropSheetThread(void* Param)
 {
+    PSERVICEPROPSHEET pServicePropSheet;
     PROPSHEETHEADER psh;
     PROPSHEETPAGE psp[4];
-    PSERVICEPROPSHEET pServicePropSheet;
     HWND hDlg = NULL;
     MSG Msg;
 
-    PMAIN_WND_INFO Info = (PMAIN_WND_INFO)Param;
+    pServicePropSheet = (PSERVICEPROPSHEET)Param;
 
     ZeroMemory(&psh, sizeof(PROPSHEETHEADER));
     psh.dwSize = sizeof(PROPSHEETHEADER);
     psh.dwFlags = PSH_PROPSHEETPAGE | PSH_PROPTITLE | PSH_MODELESS;
-    psh.hwndParent = Info->hMainWnd;
+    psh.hwndParent = pServicePropSheet->Info->hMainWnd;
     psh.hInstance = hInstance;
     psh.hIcon = LoadIcon(hInstance, MAKEINTRESOURCE(IDI_SM_ICON));
-    psh.pszCaption = Info->pCurrentService->lpDisplayName;
+    psh.pszCaption = pServicePropSheet->Info->pCurrentService->lpDisplayName;
     psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGE);
     psh.nStartPage = 0;
     psh.ppsp = psp;
 
+    /* Initialize the tabs */
+    InitPropSheetPage(&psp[0], pServicePropSheet, IDD_DLG_GENERAL, GeneralPageProc);
+    InitPropSheetPage(&psp[1], pServicePropSheet, IDD_LOGON, LogonPageProc);
+    InitPropSheetPage(&psp[2], pServicePropSheet, IDD_RECOVERY, RecoveryPageProc);
+    InitPropSheetPage(&psp[3], pServicePropSheet, IDD_DLG_DEPEND,
DependenciesPageProc);
 
-    pServicePropSheet = HeapAlloc(ProcessHeap,
-                                  0,
-                                  sizeof(*pServicePropSheet));
-    if (pServicePropSheet)
+    hDlg = (HWND)PropertySheetW(&psh);
+    if (hDlg)
     {
-        /* Save current service, as it could change while the dialog is open */
-        pServicePropSheet->pService = Info->pCurrentService;
-        pServicePropSheet->Info = Info;
-
-        InitPropSheetPage(&psp[0], pServicePropSheet, IDD_DLG_GENERAL,
GeneralPageProc);
-        InitPropSheetPage(&psp[1], pServicePropSheet, IDD_LOGON, LogonPageProc);
-        InitPropSheetPage(&psp[2], pServicePropSheet, IDD_RECOVERY,
RecoveryPageProc);
-        InitPropSheetPage(&psp[3], pServicePropSheet, IDD_DLG_DEPEND,
DependenciesPageProc);
-
-        hDlg = (HWND)PropertySheetW(&psh);
-        if (hDlg)
+        /* Pump the message queue */
+        while (GetMessageW(&Msg, NULL, 0, 0))
         {
-            /* Pump the message queue */
-            while (GetMessageW(&Msg, NULL, 0, 0))
+            if (PropSheet_GetCurrentPageHwnd(hDlg) == NULL)
             {
+                /* The user hit the ok / cancel button, pull it down */
+                EnableWindow(pServicePropSheet->Info->hMainWnd, TRUE);
+                DestroyWindow(hDlg);
+            }
 
-                if (PropSheet_GetCurrentPageHwnd(hDlg) == NULL)
-                {
-                    /* The user hit the ok / cancel button, pull it down */
-                    EnableWindow(Info->hMainWnd, TRUE);
-                    DestroyWindow(hDlg);
-                }
-
-                if (PropSheet_IsDialogMessage(hDlg, &Msg) != 0)
-                {
-                    TranslateMessage(&Msg);
-                    DispatchMessageW(&Msg);
-                }
+            if (PropSheet_IsDialogMessage(hDlg, &Msg) != 0)
+            {
+                TranslateMessage(&Msg);
+                DispatchMessageW(&Msg);
             }
         }
-
-        HeapFree(GetProcessHeap(), 0, pServicePropSheet);
     }
 
+    HeapFree(GetProcessHeap(), 0, pServicePropSheet);
+
     return (hDlg != NULL);
 }
 
diff --git a/base/applications/mscutils/servman/propsheet_depends.c
b/base/applications/mscutils/servman/propsheet_depends.c
index b32111700e..de1d6a5573 100644
--- a/base/applications/mscutils/servman/propsheet_depends.c
+++ b/base/applications/mscutils/servman/propsheet_depends.c
@@ -159,20 +159,20 @@ TreeView_GetItemText(HWND hTreeView,
 */
 
 static VOID
-InitDependPage(PSERVICEPROPSHEET pDlgInfo)
+InitDependPage(PDEPENDDATA pDependData)
 {
     /* Initialize the image list */
-    pDlgInfo->hDependsImageList = InitImageList(IDI_NODEPENDS,
-                                                IDI_DRIVER,
-                                                GetSystemMetrics(SM_CXSMICON),
-                                                GetSystemMetrics(SM_CXSMICON),
-                                                IMAGE_ICON);
+    pDependData->hDependsImageList = InitImageList(IDI_NODEPENDS,
+                                                   IDI_DRIVER,
+                                                   GetSystemMetrics(SM_CXSMICON),
+                                                   GetSystemMetrics(SM_CXSMICON),
+                                                   IMAGE_ICON);
 
     /* Set the first tree view */
-    TV1_Initialize(pDlgInfo, pDlgInfo->pService->lpServiceName);
+    TV1_Initialize(pDependData,
pDependData->pDlgInfo->pService->lpServiceName);
 
     /* Set the second tree view */
-    TV2_Initialize(pDlgInfo, pDlgInfo->pService->lpServiceName);
+    TV2_Initialize(pDependData,
pDependData->pDlgInfo->pService->lpServiceName);
 }
 
 /*
@@ -185,12 +185,13 @@ DependenciesPageProc(HWND hwndDlg,
                      WPARAM wParam,
                      LPARAM lParam)
 {
-    PSERVICEPROPSHEET pDlgInfo;
+    
+    PDEPENDDATA pDependData;
 
     /* Get the window context */
-    pDlgInfo = (PSERVICEPROPSHEET)GetWindowLongPtr(hwndDlg,
-                                                   GWLP_USERDATA);
-    if (pDlgInfo == NULL && uMsg != WM_INITDIALOG)
+    pDependData = (PDEPENDDATA)GetWindowLongPtr(hwndDlg,
+                                                GWLP_USERDATA);
+    if (pDependData == NULL && uMsg != WM_INITDIALOG)
     {
         return FALSE;
     }
@@ -199,16 +200,17 @@ DependenciesPageProc(HWND hwndDlg,
     {
         case WM_INITDIALOG:
         {
-            pDlgInfo = (PSERVICEPROPSHEET)(((LPPROPSHEETPAGE)lParam)->lParam);
-            if (pDlgInfo != NULL)
+            pDependData = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
sizeof(DEPENDDATA));
+            if (pDependData != NULL)
             {
                 SetWindowLongPtr(hwndDlg,
                                  GWLP_USERDATA,
-                                 (LONG_PTR)pDlgInfo);
+                                 (LONG_PTR)pDependData);
 
-                pDlgInfo->hDependsWnd = hwndDlg;
+                pDependData->pDlgInfo =
(PSERVICEPROPSHEET)(((LPPROPSHEETPAGE)lParam)->lParam);
+                pDependData->hDependsWnd = hwndDlg;
 
-                InitDependPage(pDlgInfo);
+                InitDependPage(pDependData);
             }
         }
         break;
@@ -226,19 +228,19 @@ DependenciesPageProc(HWND hwndDlg,
                         if (lpnmtv->hdr.idFrom == IDC_DEPEND_TREE1)
                         {
                             /* Has this node been expanded before */
-                            if (!TreeView_GetChild(pDlgInfo->hDependsTreeView1,
lpnmtv->itemNew.hItem))
+                            if (!TreeView_GetChild(pDependData->hDependsTreeView1,
lpnmtv->itemNew.hItem))
                             {
                                 /* It's not, add the children */
-                                TV1_AddDependantsToTree(pDlgInfo,
lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam);
+                                TV1_AddDependantsToTree(pDependData,
lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam);
                             }
                         }
                         else if (lpnmtv->hdr.idFrom == IDC_DEPEND_TREE2)
                         {
                             /* Has this node been expanded before */
-                            if (!TreeView_GetChild(pDlgInfo->hDependsTreeView2,
lpnmtv->itemNew.hItem))
+                            if (!TreeView_GetChild(pDependData->hDependsTreeView2,
lpnmtv->itemNew.hItem))
                             {
                                 /* It's not, add the children */
-                                TV2_AddDependantsToTree(pDlgInfo,
lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam);
+                                TV2_AddDependantsToTree(pDependData,
lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam);
                             }
                         }
                     }
@@ -256,11 +258,13 @@ DependenciesPageProc(HWND hwndDlg,
             break;
 
         case WM_DESTROY:
-            DestroyTreeView(pDlgInfo->hDependsTreeView1);
-            DestroyTreeView(pDlgInfo->hDependsTreeView2);
+            DestroyTreeView(pDependData->hDependsTreeView1);
+            DestroyTreeView(pDependData->hDependsTreeView2);
 
-            if (pDlgInfo->hDependsImageList)
-                ImageList_Destroy(pDlgInfo->hDependsImageList);
+            if (pDependData->hDependsImageList)
+                ImageList_Destroy(pDependData->hDependsImageList);
+
+            HeapFree(GetProcessHeap(), 0, pDependData);
     }
 
     return FALSE;
diff --git a/base/applications/mscutils/servman/propsheet_recovery.c
b/base/applications/mscutils/servman/propsheet_recovery.c
index 8e16122151..59205f24c0 100644
--- a/base/applications/mscutils/servman/propsheet_recovery.c
+++ b/base/applications/mscutils/servman/propsheet_recovery.c
@@ -170,7 +170,8 @@ ShowFailureActions(
 {
     WCHAR szBuffer[256];
     PWSTR startPtr, endPtr;
-    INT i, index, id, length;
+    INT index, id, length;
+    DWORD i;
 
     for (i = 0; i < min(pRecoveryData->pServiceFailure->cActions, 3); i++)
     {