From 656342ff11dd958a66af886f30afb82bbfb65747 Mon Sep 17 00:00:00 2001 From: Ged Murphy Date: Fri, 26 Jun 2015 08:45:49 +0000 Subject: [PATCH] [DEVMGR] - Add support for selecting any device when refreshing the view. This allows us to re-select a device after it's been enabled instead of collapsing the tree and losing track of what you did. - Plug some memory leaks - HeapAlloc -> new svn path=/trunk/; revision=68272 --- .../dll/win32/devmgr/devmgmt/DeviceNode.cpp | 78 +++++------ reactos/dll/win32/devmgr/devmgmt/DeviceNode.h | 3 + .../dll/win32/devmgr/devmgmt/DeviceView.cpp | 132 ++++++++++++------ reactos/dll/win32/devmgr/devmgmt/DeviceView.h | 17 ++- .../dll/win32/devmgr/devmgmt/MainWindow.cpp | 8 +- reactos/dll/win32/devmgr/devmgmt/Node.cpp | 15 -- 6 files changed, 148 insertions(+), 105 deletions(-) diff --git a/reactos/dll/win32/devmgr/devmgmt/DeviceNode.cpp b/reactos/dll/win32/devmgr/devmgmt/DeviceNode.cpp index 7695f5fc9d1..a4f46a8c49f 100644 --- a/reactos/dll/win32/devmgr/devmgmt/DeviceNode.cpp +++ b/reactos/dll/win32/devmgr/devmgmt/DeviceNode.cpp @@ -18,6 +18,7 @@ CDeviceNode::CDeviceNode( ) : CNode(ImageListData), m_DevInst(Device), + m_hDevInfo(NULL), m_Status(0), m_ProblemNumber(0), m_OverlayImage(0) @@ -27,7 +28,7 @@ CDeviceNode::CDeviceNode( CDeviceNode::~CDeviceNode() { - SetupDiDestroyDeviceInfoList(m_hDevInfo); + Cleanup(); } bool @@ -37,37 +38,31 @@ CDeviceNode::SetupNode() ULONG ulLength; CONFIGRET cr; - // ATLASSERT(m_DeviceId == NULL); - - // Get the length of the device id string cr = CM_Get_Device_ID_Size(&ulLength, m_DevInst, 0); if (cr == CR_SUCCESS) { // We alloc heap here because this will be stored in the lParam of the TV - m_DeviceId = (LPWSTR)HeapAlloc(GetProcessHeap(), - 0, - (ulLength + 1) * sizeof(WCHAR)); - if (m_DeviceId) + m_DeviceId = new WCHAR[ulLength + 1]; + + // Now get the actual device id + cr = CM_Get_Device_IDW(m_DevInst, + m_DeviceId, + ulLength + 1, + 0); + if (cr != CR_SUCCESS) { - // Now get the actual device id - cr = CM_Get_Device_IDW(m_DevInst, - m_DeviceId, - ulLength + 1, - 0); - if (cr != CR_SUCCESS) - { - HeapFree(GetProcessHeap(), 0, m_DeviceId); - m_DeviceId = NULL; - } + delete[] m_DeviceId; + m_DeviceId = NULL; } + } // Make sure we got the string if (m_DeviceId == NULL) return false; - //SP_DEVINFO_DATA DevinfoData; + // Build up a handle a and devinfodata struct m_hDevInfo = SetupDiCreateDeviceInfoListExW(NULL, NULL, NULL, @@ -83,28 +78,14 @@ CDeviceNode::SetupNode() } - - // Get the current status of the device - cr = CM_Get_DevNode_Status_Ex(&m_Status, - &m_ProblemNumber, - m_DevInst, - 0, - NULL); - if (cr != CR_SUCCESS) - { - HeapFree(GetProcessHeap(), 0, m_DeviceId); - m_DeviceId = NULL; - return false; - } - - // Check if the device has a problem - if (m_Status & DN_HAS_PROBLEM) + // Set the overlay if the device has a problem + if (HasProblem()) { m_OverlayImage = 1; } // The disabled overlay takes precidence over the problem overlay - if (m_ProblemNumber & (CM_PROB_DISABLED | CM_PROB_HARDWARE_DISABLED)) + if (IsDisabled()) { m_OverlayImage = 2; } @@ -158,11 +139,11 @@ CDeviceNode::SetupNode() // Cleanup if something failed if (cr != CR_SUCCESS) { - HeapFree(GetProcessHeap(), 0, m_DeviceId); - m_DeviceId = NULL; + Cleanup(); + return false; } - return (cr == CR_SUCCESS ? true : false); + return true; } bool @@ -340,7 +321,7 @@ CDeviceNode::EnableDevice( if (Enable) { - // config specific enablling first, then global enabling. + // config specific enabling first, then global enabling. // The global appears to be the one that starts the device pcp.Scope = DICS_FLAG_GLOBAL; if (SetupDiSetClassInstallParamsW(m_hDevInfo, @@ -362,6 +343,23 @@ CDeviceNode::EnableDevice( return true; } +/* PRIVATE METHODS ******************************************************/ + +void +CDeviceNode::Cleanup() +{ + if (m_DeviceId) + { + delete[] m_DeviceId; + m_DeviceId = NULL; + } + if (m_hDevInfo) + { + SetupDiDestroyDeviceInfoList(m_hDevInfo); + m_hDevInfo = NULL; + } +} + DWORD CDeviceNode::GetFlags( ) diff --git a/reactos/dll/win32/devmgr/devmgmt/DeviceNode.h b/reactos/dll/win32/devmgr/devmgmt/DeviceNode.h index 8e559d4fbd5..190a4444439 100644 --- a/reactos/dll/win32/devmgr/devmgmt/DeviceNode.h +++ b/reactos/dll/win32/devmgr/devmgmt/DeviceNode.h @@ -38,6 +38,9 @@ public: ); private: + void Cleanup( + ); + bool SetFlags( _In_ DWORD Flags, _In_ DWORD FlagsEx diff --git a/reactos/dll/win32/devmgr/devmgmt/DeviceView.cpp b/reactos/dll/win32/devmgr/devmgmt/DeviceView.cpp index b6ef077195c..003fad434f5 100644 --- a/reactos/dll/win32/devmgr/devmgmt/DeviceView.cpp +++ b/reactos/dll/win32/devmgr/devmgmt/DeviceView.cpp @@ -36,6 +36,7 @@ struct RefreshThreadData CDeviceView *This; BOOL ScanForChanges; BOOL UpdateView; + LPWSTR DeviceId; }; @@ -198,7 +199,8 @@ void CDeviceView::Refresh( _In_ ViewType Type, _In_ bool ScanForChanges, - _In_ bool UpdateView + _In_ bool UpdateView, + _In_opt_ LPWSTR DeviceId ) { // Enum devices on a seperate thread to keep the gui responsive @@ -210,6 +212,16 @@ CDeviceView::Refresh( ThreadData->This = this; ThreadData->ScanForChanges = ScanForChanges; ThreadData->UpdateView = UpdateView; + ThreadData->DeviceId = NULL; + + if (DeviceId) + { + // Node gets deleted on refresh so we copy it to another block + size_t Length = wcslen(DeviceId) + 1; + ThreadData->DeviceId = new WCHAR[Length]; + wcscpy_s(ThreadData->DeviceId, Length, DeviceId); + } + HANDLE hThread; hThread = (HANDLE)_beginthreadex(NULL, @@ -309,29 +321,29 @@ CDeviceView::EnableSelectedDevice( ) { CDeviceNode *Node = dynamic_cast(GetSelectedNode()); - if (Node) + if (Node == nullptr) return false; + + if (Enable == false) { - if (Enable == false) + CAtlStringW str; + if (str.LoadStringW(g_hInstance, IDS_CONFIRM_DISABLE)) { - CAtlStringW str; - if (str.LoadStringW(g_hInstance, IDS_CONFIRM_DISABLE)) + if (MessageBoxW(m_hMainWnd, + str, + Node->GetDisplayName(), + MB_YESNO | MB_ICONWARNING | MB_DEFBUTTON2) != IDYES) { - if (MessageBoxW(m_hMainWnd, - str, - Node->GetDisplayName(), - MB_YESNO | MB_ICONWARNING | MB_DEFBUTTON2) != IDYES) - { - return false; - } + return false; } } - - if (Node->EnableDevice(Enable, NeedsReboot)) - { - Refresh(m_ViewType, true, true); - return true; - } } + + if (Node->EnableDevice(Enable, NeedsReboot)) + { + Refresh(m_ViewType, true, true, Node->GetDeviceId()); + return true; + } + return false; } @@ -462,6 +474,11 @@ unsigned int __stdcall CDeviceView::RefreshThread(void *Param) break; } + + This->SelectNode(ThreadData->DeviceId); + + if (ThreadData->DeviceId) + delete[] ThreadData->DeviceId; delete ThreadData; return 0; @@ -650,7 +667,7 @@ CDeviceView::RecurseChildDevices( // Get the cached device node CDeviceNode *DeviceNode; DeviceNode = dynamic_cast(GetDeviceNode(Device)); - if (DeviceNode == NULL) + if (DeviceNode == nullptr) { ATLASSERT(FALSE); return false; @@ -684,7 +701,7 @@ CDeviceView::RecurseChildDevices( if (bSuccess == FALSE) break; DeviceNode = dynamic_cast(GetDeviceNode(Device)); - if (DeviceNode == NULL) + if (DeviceNode == nullptr) { ATLASSERT(FALSE); } @@ -790,17 +807,20 @@ CDeviceView::InsertIntoTreeView( return TreeView_InsertItem(m_hTreeView, &tvins); } -void -CDeviceView::RecurseDeviceView( - _In_ HTREEITEM hParentItem +HTREEITEM +CDeviceView::RecurseFindDevice( + _In_ HTREEITEM hParentItem, + _In_ LPWSTR DeviceId ) { + HTREEITEM FoundItem; HTREEITEM hItem; TVITEMW tvItem; + CNode *Node; // Check if this node has any children hItem = TreeView_GetChild(m_hTreeView, hParentItem); - if (hItem == NULL) return; + if (hItem == NULL) return NULL; // The lParam contains the node pointer data tvItem.hItem = hItem; @@ -808,12 +828,17 @@ CDeviceView::RecurseDeviceView( if (TreeView_GetItem(m_hTreeView, &tvItem) && tvItem.lParam != NULL) { - // Delete the node class - //delete reinterpret_cast(tvItem.lParam); + Node = reinterpret_cast(tvItem.lParam); + if (Node->GetDeviceId() && + (wcscmp(Node->GetDeviceId(), DeviceId) == 0)) + { + return hItem; + } } // This node may have its own children - RecurseDeviceView(hItem); + FoundItem = RecurseFindDevice(hItem, DeviceId); + if (FoundItem) return FoundItem; // Delete all the siblings for (;;) @@ -827,12 +852,45 @@ CDeviceView::RecurseDeviceView( tvItem.mask = TVIF_PARAM; if (TreeView_GetItem(m_hTreeView, &tvItem)) { - //if (tvItem.lParam != NULL) - // delete reinterpret_cast(tvItem.lParam); + Node = reinterpret_cast(tvItem.lParam); + if (Node->GetDeviceId() && + wcscmp(Node->GetDeviceId(), DeviceId) == 0) + { + return hItem; + } } // This node may have its own children - RecurseDeviceView(hItem); + FoundItem = RecurseFindDevice(hItem, DeviceId); + if (FoundItem) return FoundItem; + } + + return hItem; +} + +void +CDeviceView::SelectNode( + _In_ LPWSTR DeviceId + ) +{ + HTREEITEM hRoot, hItem; + + // Check if there are any items in the tree + hRoot = TreeView_GetRoot(m_hTreeView); + if (hRoot == NULL) return; + + if (DeviceId) + { + hItem = RecurseFindDevice(hRoot, DeviceId); + if (hItem) + { + TreeView_SelectItem(m_hTreeView, hItem); + TreeView_Expand(m_hTreeView, hItem, TVM_EXPAND); + } + } + else + { + TreeView_SelectItem(m_hTreeView, hRoot); } } @@ -840,22 +898,10 @@ CDeviceView::RecurseDeviceView( void CDeviceView::EmptyDeviceView() { - HTREEITEM hItem; - - // Check if there are any items in the tree - hItem = TreeView_GetRoot(m_hTreeView); - if (hItem == NULL) return; - - // Free all the class nodes - //RecurseDeviceView(hItem); - - // Delete all the items (VOID)TreeView_DeleteAllItems(m_hTreeView); } - - CClassNode* CDeviceView::GetClassNode( _In_ LPGUID ClassGuid @@ -972,6 +1018,8 @@ CDeviceView::RefreshDeviceList() { m_ClassNodeList.AddTail(ClassNode); } + + SetupDiDestroyDeviceInfoList(hDevInfo); } ClassIndex++; } while (Success); diff --git a/reactos/dll/win32/devmgr/devmgmt/DeviceView.h b/reactos/dll/win32/devmgr/devmgmt/DeviceView.h index 05aa8d70133..fc5d29f646b 100644 --- a/reactos/dll/win32/devmgr/devmgmt/DeviceView.h +++ b/reactos/dll/win32/devmgr/devmgmt/DeviceView.h @@ -55,7 +55,8 @@ public: VOID Refresh( _In_ ViewType Type, _In_ bool ScanForChanges, - _In_ bool UpdateView + _In_ bool UpdateView, + _In_opt_ LPWSTR DeviceId ); VOID DisplayPropertySheet(); @@ -127,11 +128,16 @@ private: _In_ CNode *Node ); - VOID RecurseDeviceView( - _In_ HTREEITEM hParentItem + HTREEITEM RecurseFindDevice( + _In_ HTREEITEM hParentItem, + _In_ LPWSTR DeviceId ); - VOID EmptyDeviceView( + void SelectNode( + _In_ LPWSTR DeviceId + ); + + void EmptyDeviceView( ); CNode* GetNode( @@ -145,6 +151,7 @@ private: CDeviceNode* GetDeviceNode( _In_ DEVINST Device ); - void EmptyLists(); + void EmptyLists( + ); }; diff --git a/reactos/dll/win32/devmgr/devmgmt/MainWindow.cpp b/reactos/dll/win32/devmgr/devmgmt/MainWindow.cpp index 6bdb925d4ba..058f3a7d29f 100644 --- a/reactos/dll/win32/devmgr/devmgmt/MainWindow.cpp +++ b/reactos/dll/win32/devmgr/devmgmt/MainWindow.cpp @@ -212,7 +212,7 @@ CMainWindow::RefreshView(ViewType Type) BOOL bSuccess; // Refreshed the cached view - m_DeviceView->Refresh(Type, FALSE, TRUE); + m_DeviceView->Refresh(Type, FALSE, TRUE, NULL); // Get the menu item id switch (Type) @@ -240,7 +240,8 @@ CMainWindow::ScanForHardwareChanges() // Refresh the cache and and display m_DeviceView->Refresh(m_DeviceView->GetCurrentView(), true, - true); + true, + NULL); return true; } @@ -618,7 +619,8 @@ CMainWindow::OnCommand(WPARAM wParam, // Refresh the device view m_DeviceView->Refresh(m_DeviceView->GetCurrentView(), false, - true); + true, + NULL); break; } diff --git a/reactos/dll/win32/devmgr/devmgmt/Node.cpp b/reactos/dll/win32/devmgr/devmgmt/Node.cpp index be54a0b087a..e189aa416ee 100644 --- a/reactos/dll/win32/devmgr/devmgmt/Node.cpp +++ b/reactos/dll/win32/devmgr/devmgmt/Node.cpp @@ -24,19 +24,4 @@ CNode::CNode(_In_ PSP_CLASSIMAGELIST_DATA ImageListData) : CNode::~CNode() { - Cleanup(); -} - - -/* PRIVATE METHODS ******************************************/ - - -void -CNode::Cleanup() -{ - if (m_DeviceId) - { - HeapFree(GetProcessHeap(), 0, m_DeviceId); - m_DeviceId = NULL; - } }