Changeset 30860 in vbox for trunk/src

Timestamp:

Jul 15, 2010 5:19:32 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

63689

Message:

GMMR0.cpp: fixed impossible leak in GMMR0SeekChunk. nits.

File:

• trunk/src/VBox/VMM/VMMR0/GMMR0.cpp (modified) (10 diffs)

trunk/src/VBox/VMM/VMMR0/GMMR0.cpp

@@ -489 +489 @@
 
     /** Shared module tree (global). */
-    /** todo seperate trees for distinctly different guest OSes. */
+    /** @todo seperate trees for distinctly different guest OSes. */
     PAVLGCPTRNODECORE   pGlobalSharedModuleTree;
 
@@ -867 +867 @@
              * pages will be 'left behind'.)
              */
-            /* todo this might be kind of expensive with a lot of VMs and memory hanging around... */
+            /** @todo this might be kind of expensive with a lot of VMs and
+             *   memory hanging around... */
             uint64_t cPrivatePages = pGVM->gmm.s.cPrivatePages; /* save */
             RTAvlU32DoWithAll(&pGMM->pChunks, true /* fFromLeft */, gmmR0CleanupVMScanChunk, pGVM);
@@ -3460 +3461 @@
         /* Grab the lock. */
         rc = RTSemFastMutexRequest(pGMM->Mtx);
-        AssertRCReturn(rc, rc);
-
-        /*
-         * Add a new chunk with our hGVM.
-         */
-        rc = gmmR0RegisterChunk(pGMM, &pGMM->Private, MemObj, pGVM->hSelf, GMMCHUNKTYPE_NON_CONTINUOUS);
-        RTSemFastMutexRelease(pGMM->Mtx);
+        AssertRC(rc);
+        if (RT_SUCCESS(rc))
+        {
+            /*
+             * Add a new chunk with our hGVM.
+             */
+            rc = gmmR0RegisterChunk(pGMM, &pGMM->Private, MemObj, pGVM->hSelf, GMMCHUNKTYPE_NON_CONTINUOUS);
+            RTSemFastMutexRelease(pGMM->Mtx);
+        }
 
         if (RT_FAILURE(rc))
@@ -3494 +3497 @@
     if (    pInfo
         &&  pInfo->enmGuestOS == pModule->enmGuestOS
-        /** todo replace with RTStrNCmp */
+        /** @todo replace with RTStrNCmp */
         &&  !strcmp(pModule->szName, pInfo->pszModuleName)
         &&  !strcmp(pModule->szVersion, pInfo->pszVersion))
@@ -3617 +3620 @@
             pGlobalModule->cbModule = cbModule;
             /* Input limit already safe; no need to check again. */
-            /** todo replace with RTStrCopy */
+            /** @todo replace with RTStrCopy */
             strcpy(pGlobalModule->szName, pszModuleName);
             strcpy(pGlobalModule->szVersion, pszVersion);
@@ -3649 +3652 @@
 
             /* Make sure the name and version are identical. */
-            /** todo replace with RTStrNCmp */
+            /** @todo replace with RTStrNCmp */
             if (    !strcmp(pGlobalModule->szName, pszModuleName)
                 &&  !strcmp(pGlobalModule->szVersion, pszVersion))
@@ -3743 +3746 @@
     {
         PGMMSHAREDMODULEPERVM pRecVM = (PGMMSHAREDMODULEPERVM)RTAvlGCPtrGet(&pGVM->gmm.s.pSharedModuleTree, GCBaseAddr);
-        if (!pRecVM)
-        {
-            rc = VERR_PGM_SHARED_MODULE_NOT_FOUND;
-            goto end;
-        }
-        /* Remove reference to global shared module. */
-        if (!pRecVM->fCollision)
-        {
-            PGMMSHAREDMODULE pRec = pRecVM->pGlobalModule;
-            Assert(pRec);
-
-            if (pRec)   /* paranoia */
+        if (pRecVM)
+        {
+            /* Remove reference to global shared module. */
+            if (!pRecVM->fCollision)
             {
-                Assert(pRec->cUsers);
-                pRec->cUsers--;
-                if (pRec->cUsers == 0)
+                PGMMSHAREDMODULE pRec = pRecVM->pGlobalModule;
+                Assert(pRec);
+
+                if (pRec)   /* paranoia */
                 {
-                    /* Free the ranges, but leave the pages intact as there might still be references; they will be cleared by the COW mechanism. */
-                    for (unsigned i = 0; i < pRec->cRegions; i++)
-                        if (pRec->aRegions[i].paHCPhysPageID)
-                            RTMemFree(pRec->aRegions[i].paHCPhysPageID);
-
-                    /* Remove from the tree and free memory. */
-                    RTAvlGCPtrRemove(&pGMM->pGlobalSharedModuleTree, GCBaseAddr);
-                    RTMemFree(pRec);
+                    Assert(pRec->cUsers);
+                    pRec->cUsers--;
+                    if (pRec->cUsers == 0)
+                    {
+                        /* Free the ranges, but leave the pages intact as there might still be references; they will be cleared by the COW mechanism. */
+                        for (unsigned i = 0; i < pRec->cRegions; i++)
+                            if (pRec->aRegions[i].paHCPhysPageID)
+                                RTMemFree(pRec->aRegions[i].paHCPhysPageID);
+
+                        /* Remove from the tree and free memory. */
+                        RTAvlGCPtrRemove(&pGMM->pGlobalSharedModuleTree, GCBaseAddr);
+                        RTMemFree(pRec);
+                    }
                 }
+                else
+                    rc = VERR_PGM_SHARED_MODULE_REGISTRATION_INCONSISTENCY;
             }
             else
-                rc = VERR_PGM_SHARED_MODULE_REGISTRATION_INCONSISTENCY;
+                Assert(!pRecVM->pGlobalModule);
+
+            /* Remove from the tree and free memory. */
+            RTAvlGCPtrRemove(&pGVM->gmm.s.pSharedModuleTree, GCBaseAddr);
+            RTMemFree(pRecVM);
         }
         else
-            Assert(!pRecVM->pGlobalModule);
-
-        /* Remove from the tree and free memory. */
-        RTAvlGCPtrRemove(&pGVM->gmm.s.pSharedModuleTree, GCBaseAddr);
-        RTMemFree(pRecVM);
+            rc = VERR_PGM_SHARED_MODULE_NOT_FOUND;
 
         GMM_CHECK_SANITY_UPON_LEAVING(pGMM);
@@ -3785 +3788 @@
         rc = VERR_INTERNAL_ERROR_5;
 
-end:
     RTSemFastMutexRelease(pGMM->Mtx);
     return rc;
@@ -3904 +3906 @@
         {
             /* Page was freed at some point; invalidate this entry. */
-            /** todo this isn't really bullet proof. */
+            /** @todo this isn't really bullet proof. */
             Log(("Old shared page was freed -> create a new one\n"));
             pGlobalRegion->paHCPhysPageID[idxPage] = NIL_GMM_PAGEID;
@@ -3948 +3950 @@
         pbSharedPage = pbChunk + ((pGlobalRegion->paHCPhysPageID[idxPage] & GMM_PAGEID_IDX_MASK) << PAGE_SHIFT);
 
-        /** todo write ASMMemComparePage. */
+        /** @todo write ASMMemComparePage. */
         if (memcmp(pbSharedPage, pbLocalPage, PAGE_SIZE))
         {