Changeset 83569 in vbox for trunk/src

Timestamp:

Apr 5, 2020 9:23:26 PM (5 years ago)

Author:

vboxsync

Message:

DevVirtioSCSI.cpp: Eliminated two small memory allocations in virtioScsiR3IoReqFinish. bugref:9440

File:

• trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (modified) (2 diffs)

trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp

@@ -975 +975 @@
 
         /* req datain bytes already in guest phys mem. via virtioScsiIoReqCopyFromBuf() */
-        /** @todo r=bird: There is too much allocating here and we'll leak stuff if
-         *        we're low on memory and one of the RTMemAllocZ calls fail! */
-
-        PRTSGBUF pReqSegBuf = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF));
-        AssertReturn(pReqSegBuf, VERR_NO_MEMORY);
-
-        PRTSGSEG paReqSegs  = (PRTSGSEG)RTMemAllocZ(sizeof(RTSGSEG) * 2);
-        AssertReturn(paReqSegs, VERR_NO_MEMORY);
-
-        int cSegs = 0;
-        paReqSegs[cSegs].pvSeg = &respHdr;
-        paReqSegs[cSegs++].cbSeg = sizeof(respHdr);
-
-        paReqSegs[cSegs].pvSeg = pReq->pbSense;
-        paReqSegs[cSegs++].cbSeg = pReq->cbSenseAlloc; /* VirtIO 1.0 spec 5.6.4/5.6.6.1 */
-
-        /* Copy segment data to malloc'd memory to avoid stack out-of-scope errors sanitizer doesn't detect */
-        /** @todo r=bird: The above comment makes zero sense as the memory is freed
-         *        before we return, so there cannot be any trouble with out-of-scope
-         *        stuff here. */
-        for (int i = 0; i < cSegs; i++)
-        {
-            void *pv = paReqSegs[i].pvSeg;
-            paReqSegs[i].pvSeg = RTMemDup(pv, paReqSegs[i].cbSeg);
-            AssertReturn(paReqSegs[i].pvSeg, VERR_NO_MEMORY);
-        }
-
-        RTSgBufInit(pReqSegBuf, paReqSegs, cSegs);
-
-        size_t cbReqSgBuf = RTSgBufCalcTotalLength(pReqSegBuf);
+        RTSGSEG aReqSegs[2];
+
+        paReqSegs[0].pvSeg = &respHdr;
+        paReqSegs[0].cbSeg = sizeof(respHdr);
+
+        paReqSegs[1].pvSeg = pReq->pbSense;
+        paReqSegs[1].cbSeg = pReq->cbSenseAlloc; /* VirtIO 1.0 spec 5.6.4/5.6.6.1 */
+
+        RTSGBUF ReqSgBuf
+        RTSgBufInit(&ReqSgBuf, aReqSegs, RT_ELEMENTS(aRegSegs));
+
+        size_t cbReqSgBuf = RTSgBufCalcTotalLength(&ReqSgBuf);
+        /** @todo r=bird: Returning here looks a little bogus... */
         AssertMsgReturn(cbReqSgBuf <= pReq->pDescChain->cbPhysReturn,
-                       ("Guest expected less req data (space needed: %d, avail: %d)\n",
-                         cbReqSgBuf, pReq->pDescChain->cbPhysReturn),
-                       VERR_BUFFER_OVERFLOW);
-
-        virtioCoreR3QueuePut(pDevIns, &pThis->Virtio, pReq->qIdx, pReqSegBuf, pReq->pDescChain, true /* fFence TBD */);
+                       ("Guest expected less req data (space needed: %zu, avail: %u)\n",
+                        cbReqSgBuf, pReq->pDescChain->cbPhysReturn),
+                        VERR_BUFFER_OVERFLOW);
+
+        virtioCoreR3QueuePut(pDevIns, &pThis->Virtio, pReq->qIdx, &ReqSgBuf, pReq->pDescChain, true /* fFence TBD */);
         virtioCoreQueueSync(pDevIns, &pThis->Virtio, pReq->qIdx);
-
-        for (int i = 0; i < cSegs; i++)
-            RTMemFree(paReqSegs[i].pvSeg);
-
-        RTMemFree(paReqSegs);
-        RTMemFree(pReqSegBuf);
 
         Log2(("-----------------------------------------------------------------------------------------\n"));
@@ -1027 +1004 @@
         PDMDevHlpAsyncNotificationCompleted(pDevIns);
 
-    return VINF_SUCCESS;
+    return rc;
 }