Changeset 38714 in vbox for trunk/src/VBox/HostDrivers/VBoxUSB/win/lib/VBoxUsbLib-win.cpp

Timestamp:

Sep 12, 2011 11:58:43 AM (13 years ago)

Author:

vboxsync

Message:

VBoxUsbLib-win.cpp: Don't close hMonitor (or anything else) twice when init failed! Two r=bird comments. Cleanups.

File:

• trunk/src/VBox/HostDrivers/VBoxUSB/win/lib/VBoxUsbLib-win.cpp (modified) (18 diffs)

trunk/src/VBox/HostDrivers/VBoxUSB/win/lib/VBoxUsbLib-win.cpp

@@ -1170 +1170 @@
 }
 
-static void usbLibOnDeviceChange()
+static void usbLibOnDeviceChange(void)
 {
     /* we're getting series of events like that especially on device re-attach
@@ -1183 +1183 @@
         {
             DWORD winEr = GetLastError();
-            AssertMsg(winEr == ERROR_IO_PENDING, (__FUNCTION__": DeleteTimerQueueTimer failed, winEr (%d)\n", winEr));
+            AssertMsg(winEr == ERROR_IO_PENDING, ("DeleteTimerQueueTimer failed, winEr (%d)\n", winEr));
         }
     }
 
     if (!CreateTimerQueueTimer(&g_VBoxUsbGlobal.hTimer, g_VBoxUsbGlobal.hTimerQueue,
-                                        usbLibTimerCallback,
-                                        NULL,
-                                        500, /* ms*/
-                                        0,
-                                        WT_EXECUTEONLYONCE))
-    {
-            DWORD winEr = GetLastError();
-            AssertMsgFailed(("CreateTimerQueueTimer failed, winEr (%d)\n", winEr));
-
-            /* call it directly */
-            usbLibTimerCallback(NULL, FALSE);
-    }
-}
-
-static LRESULT CALLBACK usbLibWndProc(HWND hwnd,
-    UINT uMsg,
-    WPARAM wParam,
-    LPARAM lParam
-)
+                               usbLibTimerCallback,
+                               NULL,
+                               500, /* ms*/
+                               0,
+                               WT_EXECUTEONLYONCE))
+    {
+        DWORD winEr = GetLastError();
+        AssertMsgFailed(("CreateTimerQueueTimer failed, winEr (%d)\n", winEr));
+
+        /* call it directly */
+        usbLibTimerCallback(NULL, FALSE);
+    }
+}
+
+static LRESULT CALLBACK usbLibWndProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
 {
     switch (uMsg)
@@ -1231 +1227 @@
 }
 
-static LPCSTR g_VBoxUsbWndClassName = "VBoxUsbLibClass";
-
-static DWORD WINAPI usbLibMsgThreadProc(__in  LPVOID lpParameter)
-{
-     HWND                 hwnd = 0;
-     HINSTANCE hInstance = (HINSTANCE)GetModuleHandle (NULL);
-     bool bExit = false;
+static DWORD WINAPI usbLibMsgThreadProc(__in LPVOID lpParameter)
+{
+    static LPCSTR   s_szVBoxUsbWndClassName = "VBoxUsbLibClass";
+     HWND           hwnd      = 0;
+     HINSTANCE      hInstance = (HINSTANCE)GetModuleHandle(NULL);
+     bool           bExit     = false;
 
      /* Register the Window Class. */
@@ -1250 +1245 @@
      wc.hbrBackground = (HBRUSH)(COLOR_BACKGROUND + 1);
      wc.lpszMenuName  = NULL;
-     wc.lpszClassName = g_VBoxUsbWndClassName;
+     wc.lpszClassName = s_szVBoxUsbWndClassName;
 
      ATOM atomWindowClass = RegisterClass(&wc);
@@ -1257 +1252 @@
      {
          /* Create the window. */
-         g_VBoxUsbGlobal.hWnd = CreateWindowEx (WS_EX_TOOLWINDOW | WS_EX_TRANSPARENT | WS_EX_TOPMOST,
-                 g_VBoxUsbWndClassName, g_VBoxUsbWndClassName,
-                                                   WS_POPUPWINDOW,
-                                                  -200, -200, 100, 100, NULL, NULL, hInstance, NULL);
+         g_VBoxUsbGlobal.hWnd = CreateWindowEx(WS_EX_TOOLWINDOW | WS_EX_TRANSPARENT | WS_EX_TOPMOST,
+                                               s_szVBoxUsbWndClassName, s_szVBoxUsbWndClassName,
+                                               WS_POPUPWINDOW,
+                                               -200, -200, 100, 100, NULL, NULL, hInstance, NULL);
          SetEvent(g_VBoxUsbGlobal.hNotifyEvent);
 
@@ -1275 +1270 @@
              }
 
-             DestroyWindow (hwnd);
+             DestroyWindow(hwnd);
 
              bExit = true;
          }
 
-         UnregisterClass (g_VBoxUsbWndClassName, hInstance);
+         UnregisterClass(s_szVBoxUsbWndClassName, hInstance);
      }
 
-     if(bExit)
+     /** @todo r=bird: Please explain why the USB library needs
+      *        this exit(0) hack! */
+     if (bExit)
      {
          /* no need any accuracy here, in anyway the DHCP server usually gets terminated with TerminateProcess */
@@ -1308 +1305 @@
     g_VBoxUsbGlobal.hMonitor = INVALID_HANDLE_VALUE;
 
-    g_VBoxUsbGlobal.hNotifyEvent = CreateEvent(NULL, /* LPSECURITY_ATTRIBUTES lpEventAttributes */
-                                        FALSE, /* BOOL bManualReset */
+    g_VBoxUsbGlobal.hNotifyEvent = CreateEvent(NULL,  /* LPSECURITY_ATTRIBUTES lpEventAttributes */
+                                               FALSE, /* BOOL bManualReset */
 #ifndef VBOX_USB_USE_DEVICE_NOTIFICATION
-                                        TRUE,  /* BOOL bInitialState */
+                                               TRUE,  /* BOOL bInitialState */
 #else
-                                        FALSE, /* set to false since it will be initially used for notification thread startup sync */
-#endif
-                                        NULL /* LPCTSTR lpName */);
+                                               FALSE, /* set to false since it will be initially used for notification thread startup sync */
+#endif
+                                               NULL   /* LPCTSTR lpName */);
     if (g_VBoxUsbGlobal.hNotifyEvent)
     {
-        g_VBoxUsbGlobal.hInterruptEvent = CreateEvent(NULL, /* LPSECURITY_ATTRIBUTES lpEventAttributes */
-                                                FALSE, /* BOOL bManualReset */
-                                                FALSE,  /* BOOL bInitialState */
-                                                NULL /* LPCTSTR lpName */);
+        g_VBoxUsbGlobal.hInterruptEvent = CreateEvent(NULL,  /* LPSECURITY_ATTRIBUTES lpEventAttributes */
+                                                      FALSE, /* BOOL bManualReset */
+                                                      FALSE, /* BOOL bInitialState */
+                                                      NULL   /* LPCTSTR lpName */);
         if (g_VBoxUsbGlobal.hInterruptEvent)
         {
@@ -1350 +1347 @@
                 if (DeviceIoControl(g_VBoxUsbGlobal.hMonitor, SUPUSBFLT_IOCTL_GET_VERSION, NULL, 0, &Version, sizeof (Version), &cbReturned, NULL))
                 {
-                    if (Version.u32Major == USBMON_MAJOR_VERSION || Version.u32Minor <= USBMON_MINOR_VERSION)
+                    /** @todo r=bird: Explain why we accept any mismatching major
+                     *        version as long as the minor version is less or equal!
+                     *        It does not make sense to me... */
+                    if (   Version.u32Major == USBMON_MAJOR_VERSION 
+                        || Version.u32Minor <= USBMON_MINOR_VERSION)
                     {
 #ifndef VBOX_USB_USE_DEVICE_NOTIFICATION
@@ -1357 +1358 @@
                         SetEvent.u.hEvent = g_VBoxUsbGlobal.hNotifyEvent;
                         if (DeviceIoControl(g_VBoxUsbGlobal.hMonitor, SUPUSBFLT_IOCTL_SET_NOTIFY_EVENT,
-                                &SetEvent, sizeof (SetEvent),
-                                &SetEvent, sizeof (SetEvent),
-                                &cbReturned, NULL))
+                                            &SetEvent, sizeof(SetEvent),
+                                            &SetEvent, sizeof(SetEvent),
+                                            &cbReturned, NULL))
                         {
                             rc = SetEvent.u.rc;
                             AssertRC(rc);
                             if (RT_SUCCESS(rc))
+                            {
+                                /* 
+                                 * We're DONE!
+                                 */ 
                                 return VINF_SUCCESS;
-                            else
-                                AssertMsgFailed(("SetEvent failed, rc (%d)\n", rc));
+                            }
+                            AssertMsgFailed(("SetEvent failed, rc (%d)\n", rc));
                         }
                         else
@@ -1386 +1391 @@
                               NULL /*__out_opt  LPDWORD lpThreadId*/
                             );
-
-                            if(g_VBoxUsbGlobal.hThread)
+                            if (g_VBoxUsbGlobal.hThread)
                             {
                                 DWORD dwResult = WaitForSingleObject(g_VBoxUsbGlobal.hNotifyEvent, INFINITE);
                                 Assert(dwResult == WAIT_OBJECT_0);
-
                                 if (g_VBoxUsbGlobal.hWnd)
                                 {
-                                    /* ensure the event is set so the first "wait change" request processes */
+                                    /* 
+                                     * We're DONE!
+                                     *  
+                                     * Juse ensure that the event is set so the 
+                                     * first "wait change" request is processed.
+                                     */
                                     SetEvent(g_VBoxUsbGlobal.hNotifyEvent);
                                     return VINF_SUCCESS;
                                 }
+
                                 dwResult = WaitForSingleObject(g_VBoxUsbGlobal.hThread, INFINITE);
                                 Assert(dwResult == WAIT_OBJECT_0);
                                 BOOL bRc = CloseHandle(g_VBoxUsbGlobal.hThread);
-                                if (!bRc)
-                                {
-                                    DWORD winEr = GetLastError();
-                                    AssertMsgFailed(("CloseHandle for hThread failed winEr(%d)\n", winEr));
-                                }
+                                AssertMsg(bRc, ("CloseHandle for hThread failed winEr(%d)\n", GetLastError()));
+                                g_VBoxUsbGlobal.hThread = INVALID_HANDLE_VALUE;
                             }
                             else
@@ -1413 +1419 @@
                                 rc = VERR_GENERAL_FAILURE;
                             }
+
+                            DeleteTimerQueueEx(g_VBoxUsbGlobal.hTimerQueue, INVALID_HANDLE_VALUE /* see term */);
+                            g_VBoxUsbGlobal.hTimerQueue = NULL;
                         }
                         else
@@ -1438 +1447 @@
 
                 CloseHandle(g_VBoxUsbGlobal.hMonitor);
+                g_VBoxUsbGlobal.hMonitor = INVALID_HANDLE_VALUE;
             }
             else
@@ -1449 +1459 @@
 
             CloseHandle(g_VBoxUsbGlobal.hInterruptEvent);
+            g_VBoxUsbGlobal.hInterruptEvent = NULL;
         }
         else
@@ -1458 +1469 @@
 
         CloseHandle(g_VBoxUsbGlobal.hNotifyEvent);
+        g_VBoxUsbGlobal.hNotifyEvent = NULL;
     }
     else
@@ -1483 +1495 @@
     if (g_VBoxUsbGlobal.hMonitor == INVALID_HANDLE_VALUE)
     {
-#ifdef VBOX_WITH_ANNOYING_USB_ASSERTIONS
-        AssertFailed();
-#endif
-        return VINF_ALREADY_INITIALIZED;
+        Assert(g_VBoxUsbGlobal.hInterruptEvent == NULL);
+        Assert(g_VBoxUsbGlobal.hNotifyEvent == NULL);
+        return VINF_SUCCESS;
     }
 
     BOOL bRc;
 #ifdef VBOX_USB_USE_DEVICE_NOTIFICATION
-    bRc= PostMessage(g_VBoxUsbGlobal.hWnd, WM_QUIT, 0, 0);
+    bRc = PostMessage(g_VBoxUsbGlobal.hWnd, WM_QUIT, 0, 0);
     if (!bRc)
     {
@@ -1503 +1514 @@
         Assert(dwResult == WAIT_OBJECT_0);
         bRc = CloseHandle(g_VBoxUsbGlobal.hThread);
-        if (!bRc)
-        {
-            DWORD winEr = GetLastError();
-            AssertMsgFailed(("CloseHandle for hThread failed winEr(%d)\n", winEr));
-        }
+        AssertMsg(bRc, ("CloseHandle for hThread failed winEr(%d)\n", GetLastError()));
     }
 
@@ -1513 +1520 @@
     {
         bRc = DeleteTimerQueueTimer(g_VBoxUsbGlobal.hTimerQueue, g_VBoxUsbGlobal.hTimer,
-                INVALID_HANDLE_VALUE /* <-- to block until the timer is completed */
-                            );
-        if (!bRc)
-        {
-            DWORD winEr = GetLastError();
-            AssertMsgFailed(("DeleteTimerQueueEx failed winEr(%d)\n", winEr));
-        }
+                                    INVALID_HANDLE_VALUE); /* <-- to block until the timer is completed */
+        AssertMsg(bRc, ("DeleteTimerQueueTimer failed winEr(%d)\n", GetLastError()));
     }
 
@@ -1525 +1527 @@
     {
         bRc = DeleteTimerQueueEx(g_VBoxUsbGlobal.hTimerQueue,
-                INVALID_HANDLE_VALUE /* <-- to block until all timers are completed */
-                );
-        if (!bRc)
-        {
-            DWORD winEr = GetLastError();
-            AssertMsgFailed(("DeleteTimerQueueEx failed winEr(%d)\n", winEr));
-        }
-    }
-#endif
+                                 INVALID_HANDLE_VALUE); /* <-- to block until all timers are completed */
+        AssertMsg(bRc, ("DeleteTimerQueueEx failed winEr(%d)\n", GetLastError()));
+    }
+#endif /* VBOX_USB_USE_DEVICE_NOTIFICATION */
 
     bRc = CloseHandle(g_VBoxUsbGlobal.hMonitor);
-    if (!bRc)
-    {
-        DWORD winEr = GetLastError();
-        AssertMsgFailed(("CloseHandle for hMonitor failed winEr(%d)\n", winEr));
-    }
-
+    AssertMsg(bRc, ("CloseHandle for hMonitor failed winEr(%d)\n", GetLastError()));
+    g_VBoxUsbGlobal.hMonitor = INVALID_HANDLE_VALUE;
+          
     bRc = CloseHandle(g_VBoxUsbGlobal.hInterruptEvent);
-    if (!bRc)
-    {
-        DWORD winEr = GetLastError();
-        AssertMsgFailed(("CloseHandle for hInterruptEvent failed winEr(%d)\n", winEr));
-    }
+    AssertMsg(bRc, ("CloseHandle for hInterruptEvent failed lasterr=%u\n", GetLastError()));
+    g_VBoxUsbGlobal.hInterruptEvent = NULL;
 
     bRc = CloseHandle(g_VBoxUsbGlobal.hNotifyEvent);
-    if (!bRc)
-    {
-        DWORD winEr = GetLastError();
-        AssertMsgFailed(("CloseHandle for hNotifyEvent failed winEr(%d)\n", winEr));
-    }
+    AssertMsg(bRc, ("CloseHandle for hNotifyEvent failed winEr(%d)\n", GetLastError()));
+    g_VBoxUsbGlobal.hNotifyEvent = NULL;
 
     return VINF_SUCCESS;