Changeset 44991 in vbox

Timestamp:

Mar 11, 2013 2:55:10 PM (12 years ago)

Author:

vboxsync

Message:

VBoxGuest-win*: More review + cleanup.

Location:

trunk/src/VBox/Additions/common/VBoxGuest

Files:

• VBoxGuest-win-pnp.cpp (modified) (2 diffs)
• VBoxGuest-win.cpp (modified) (8 diffs)
• VBoxGuest-win.h (modified) (1 diff)

trunk/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win-pnp.cpp

@@ -254 +254 @@
 
             /* Free hardware resources. */
-            /* @todo this should actually free I/O ports, interrupts, etc. */
+            /** @todo this should actually free I/O ports, interrupts, etc.
+             * Update/bird: vbgdNtCleanup actually does that... So, what's there to do?  */
             rc = vbgdNtCleanup(pDevObj);
             Log(("VBoxGuest::vbgdNtGuestPnp: REMOVE_DEVICE: vbgdNtCleanup rc = 0x%08X\n", rc));
@@ -349 +350 @@
 
             /* Free hardware resources. */
-            /* @todo this should actually free I/O ports, interrupts, etc. */
+            /** @todo this should actually free I/O ports, interrupts, etc.
+             * Update/bird: vbgdNtCleanup actually does that... So, what's there to do?  */
             rc = vbgdNtCleanup(pDevObj);
             Log(("VBoxGuest::vbgdNtGuestPnp: STOP_DEVICE: cleaning up, rc = 0x%x\n", rc));

trunk/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.cpp

@@ -515 +515 @@
         pDevExt->pInterruptObject = NULL;
 
+    /** @todo r=bird: The error cleanup here is completely missing. We'll leak a
+     *        whole bunch of things... */
+
     Log(("VBoxGuest::vbgdNtInit: Returned with rc = 0x%x\n", rc));
     return rc;
@@ -582 +585 @@
      * opened, at least we'll blindly assume that here.
      */
-    UNICODE_STRING win32Name;
-    RtlInitUnicodeString(&win32Name, VBOXGUEST_DEVICE_NAME_DOS);
-    NTSTATUS rc = IoDeleteSymbolicLink(&win32Name);
+    UNICODE_STRING DosName;
+    RtlInitUnicodeString(&DosName, VBOXGUEST_DEVICE_NAME_DOS);
+    NTSTATUS rc = IoDeleteSymbolicLink(&DosName);
 
     IoDeleteDevice(pDrvObj->DeviceObject);
-#else /* TARGET_NT4 */
+#else  /* !TARGET_NT4 */
     /* On a PnP driver this routine will be called after
      * IRP_MN_REMOVE_DEVICE (where we already did the cleanup),
      * so don't do anything here (yet). */
-#endif
+#endif /* !TARGET_NT4 */
 
     Log(("VBoxGuest::vbgdNtGuestUnload: returning\n"));
@@ -1103 +1106 @@
                 {
                     Log(("VBoxGuest::vbgdNtScanPCIResourceList: I/O range: Base = %08x:%08x, Length = %08x\n",
-                            pPartialData->u.Port.Start.HighPart,
-                            pPartialData->u.Port.Start.LowPart,
-                            pPartialData->u.Port.Length));
+                         pPartialData->u.Port.Start.HighPart,
+                         pPartialData->u.Port.Start.LowPart,
+                         pPartialData->u.Port.Length));
 
                     /* Save the IO port base. */
-                    /** @todo Not so good. */
+                    /** @todo Not so good.
+                     * Update/bird: What is not so good? That we just consider the last range?  */
                     pDevExt->Core.IOPortBase = (RTIOPORT)pPartialData->u.Port.Start.LowPart;
 
@@ -1118 +1122 @@
 
                     Log(("VBoxGuest::vbgdNtScanPCIResourceList: I/O range for VMMDev found! Base = %08x:%08x, Length = %08x\n",
-                            pPartialData->u.Port.Start.HighPart,
-                            pPartialData->u.Port.Start.LowPart,
-                            pPartialData->u.Port.Length));
+                         pPartialData->u.Port.Start.HighPart,
+                         pPartialData->u.Port.Start.LowPart,
+                         pPartialData->u.Port.Length));
 
                     /* Next item ... */
@@ -1210 +1214 @@
  *
  * @param pDevExt           The device extension.
- * @param physicalAdr       Physical address to map.
- * @param ulLength          Length (in bytes) to map.
+ * @param PhysAddr          Physical address to map.
+ * @param cbToMap           Number of bytes to map.
  * @param ppvMMIOBase       Pointer of mapped I/O base.
  * @param pcbMMIO           Length of mapped I/O base.
  */
-NTSTATUS vbgdNtMapVMMDevMemory(PVBOXGUESTDEVEXTWIN pDevExt, PHYSICAL_ADDRESS physicalAdr, ULONG ulLength,
+NTSTATUS vbgdNtMapVMMDevMemory(PVBOXGUESTDEVEXTWIN pDevExt, PHYSICAL_ADDRESS PhysAddr, ULONG cbToMap,
                                void **ppvMMIOBase, uint32_t *pcbMMIO)
 {
@@ -1223 +1227 @@
 
     NTSTATUS rc = STATUS_SUCCESS;
-    if (physicalAdr.LowPart > 0) /* We're mapping below 4GB. */
-    {
-         VMMDevMemory *pVMMDevMemory = (VMMDevMemory *)MmMapIoSpace(physicalAdr, ulLength, MmNonCached);
+    if (PhysAddr.LowPart > 0) /* We're mapping below 4GB. */
+    {
+         VMMDevMemory *pVMMDevMemory = (VMMDevMemory *)MmMapIoSpace(PhysAddr, cbToMap, MmNonCached);
          Log(("VBoxGuest::vbgdNtMapVMMDevMemory: pVMMDevMemory = 0x%x\n", pVMMDevMemory));
          if (pVMMDevMemory)
@@ -1233 +1237 @@
 
              /* Check version of the structure; do we have the right memory version? */
-             if (pVMMDevMemory->u32Version != VMMDEV_MEMORY_VERSION)
-             {
-                 Log(("VBoxGuest::vbgdNtMapVMMDevMemory: Wrong version (%u), refusing operation!\n",
-                      pVMMDevMemory->u32Version));
-
-                 /* Not our version, refuse operation and unmap the memory. */
-                 vbgdNtUnmapVMMDevMemory(pDevExt);
-                 rc = STATUS_UNSUCCESSFUL;
-             }
-             else
+             if (pVMMDevMemory->u32Version == VMMDEV_MEMORY_VERSION)
              {
                  /* Save results. */
@@ -1251 +1246 @@
                  Log(("VBoxGuest::vbgdNtMapVMMDevMemory: VMMDevMemory found and mapped! pvMMIOBase = 0x%p\n",
                       *ppvMMIOBase));
+             }
+             else
+             {
+                 /* Not our version, refuse operation and unmap the memory. */
+                 Log(("VBoxGuest::vbgdNtMapVMMDevMemory: Wrong version (%u), refusing operation!\n", pVMMDevMemory->u32Version));
+                 vbgdNtUnmapVMMDevMemory(pDevExt);
+                 rc = STATUS_UNSUCCESSFUL;
              }
          }

trunk/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.h

@@ -172 +172 @@
 BOOLEAN    vbgdNtIsrHandler(PKINTERRUPT interrupt, PVOID serviceContext);
 NTSTATUS   vbgdNtScanPCIResourceList(PCM_RESOURCE_LIST pResList, PVBOXGUESTDEVEXTWIN pDevExt);
-NTSTATUS   vbgdNtMapVMMDevMemory(PVBOXGUESTDEVEXTWIN pDevExt, PHYSICAL_ADDRESS physicalAdr, ULONG ulLength,
+NTSTATUS   vbgdNtMapVMMDevMemory(PVBOXGUESTDEVEXTWIN pDevExt, PHYSICAL_ADDRESS PhysAddr, ULONG cbToMap,
                                  void **ppvMMIOBase, uint32_t *pcbMMIO);
 void       vbgdNtUnmapVMMDevMemory(PVBOXGUESTDEVEXTWIN pDevExt);