[DEVMGR]
authorGed Murphy <gedmurphy@reactos.org>
Fri, 26 Jun 2015 08:45:49 +0000 (08:45 +0000)
committerGed Murphy <gedmurphy@reactos.org>
Fri, 26 Jun 2015 08:45:49 +0000 (08:45 +0000)
- 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

reactos/dll/win32/devmgr/devmgmt/DeviceNode.cpp
reactos/dll/win32/devmgr/devmgmt/DeviceNode.h
reactos/dll/win32/devmgr/devmgmt/DeviceView.cpp
reactos/dll/win32/devmgr/devmgmt/DeviceView.h
reactos/dll/win32/devmgr/devmgmt/MainWindow.cpp
reactos/dll/win32/devmgr/devmgmt/Node.cpp

index 7695f5f..a4f46a8 100644 (file)
@@ -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(
     )
index 8e559d4..190a444 100644 (file)
@@ -38,6 +38,9 @@ public:
         );
 
 private:
+    void Cleanup(
+        );
+
     bool SetFlags(
         _In_ DWORD Flags,
         _In_ DWORD FlagsEx
index b6ef077..003fad4 100644 (file)
@@ -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<CDeviceNode *>(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<CDeviceNode *>(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<CDeviceNode *>(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<CNode *>(tvItem.lParam);
+        Node = reinterpret_cast<CNode *>(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,33 +852,54 @@ CDeviceView::RecurseDeviceView(
         tvItem.mask = TVIF_PARAM;
         if (TreeView_GetItem(m_hTreeView, &tvItem))
         {
-            //if (tvItem.lParam != NULL)
-            //    delete reinterpret_cast<CNode *>(tvItem.lParam);
+            Node = reinterpret_cast<CNode *>(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::EmptyDeviceView()
+CDeviceView::SelectNode(
+    _In_ LPWSTR DeviceId
+    )
 {
-    HTREEITEM hItem;
+    HTREEITEM hRoot, hItem;
 
     // Check if there are any items in the tree
-    hItem = TreeView_GetRoot(m_hTreeView);
-    if (hItem == NULL) return;
+    hRoot = TreeView_GetRoot(m_hTreeView);
+    if (hRoot == NULL) return;
 
-    // Free all the class nodes
-    //RecurseDeviceView(hItem);
-
-    // Delete all the items
-    (VOID)TreeView_DeleteAllItems(m_hTreeView);
+    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);
+    }
 }
 
 
+void
+CDeviceView::EmptyDeviceView()
+{
+    (VOID)TreeView_DeleteAllItems(m_hTreeView);
+}
 
 
 CClassNode*
@@ -972,6 +1018,8 @@ CDeviceView::RefreshDeviceList()
             {
                 m_ClassNodeList.AddTail(ClassNode);
             }
+
+            SetupDiDestroyDeviceInfoList(hDevInfo);
         }
         ClassIndex++;
     } while (Success);
index 05aa8d7..fc5d29f 100644 (file)
@@ -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(
+        );
 };
 
index 6bdb925..058f3a7 100644 (file)
@@ -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;
         }
 
index be54a0b..e189aa4 100644 (file)
@@ -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;
-    }
 }