Changeset 83573 in vbox for trunk/src

Timestamp:

Apr 5, 2020 10:16:38 PM (5 years ago)

Author:

vboxsync

Message:

DevVirtioSCSI.cpp: Eliminated unnecessary memory allocations in virtioScsiR3SendEvent, fixing missing virtioCoreR3QueueGet status check. bugref:9440

File:

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

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

@@ -665 +665 @@
 static int virtioScsiR3SendEvent(PPDMDEVINS pDevIns, PVIRTIOSCSI pThis, uint16_t uTarget, uint32_t uEventType, uint32_t uReason)
 {
-    VIRTIOSCSI_EVENT_T event;
-    event.uEvent = uEventType;
-    event.uReason = uReason;
-    event.abVirtioLun[0] = 1;
-    event.abVirtioLun[1] = uTarget;
-    event.abVirtioLun[2] = (LUN0 >> 8) & 0x40;
-    event.abVirtioLun[3] = LUN0 & 0xff;
-    event.abVirtioLun[4] = event.abVirtioLun[5] = event.abVirtioLun[6] = event.abVirtioLun[7] = 0;
-
     switch (uEventType)
     {
@@ -714 +705 @@
     }
 
+    /** @todo r=bird: virtioCoreQueueIsEmpty() only differs in one step from what
+     *        virtioCoreR3QueueGet() also does, and that is checking
+     *        VIRTIO_STATUS_DRIVER_OK.  So, why not combine the two and check the
+     *        virtioCoreR3QueueGet status code?!  (Actually, I've added a status
+     *        check there myself, as we'll dump core if it fails and we don't.) */
     if (virtioCoreQueueIsEmpty(pDevIns, &pThis->Virtio, EVENTQ_IDX))
     {
@@ -722 +718 @@
 
     PVIRTIO_DESC_CHAIN_T pDescChain;
-    virtioCoreR3QueueGet(pDevIns, &pThis->Virtio, EVENTQ_IDX, &pDescChain, true);
-
-    PRTSGBUF pReqSegBuf = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF));
-    AssertReturn(pReqSegBuf, VERR_NO_MEMORY);
-
-    PRTSGSEG paReqSegs  = (PRTSGSEG)RTMemAllocZ(sizeof(RTSGSEG) * 2);
-    AssertReturn(paReqSegs, VERR_NO_MEMORY);
-
-    /* Copy segment data to malloc'd memory to avoid stack out-of-scope errors sanitizer doesn't detect */
-    paReqSegs[0].pvSeg = RTMemAlloc(sizeof(event));
-    AssertReturn(paReqSegs[0].pvSeg, VERR_NO_MEMORY);
-    memcpy(paReqSegs[0].pvSeg, &event, sizeof(event));
-
-    RTSgBufInit(pReqSegBuf, paReqSegs, 1);
-
-    virtioCoreR3QueuePut(pDevIns, &pThis->Virtio, EVENTQ_IDX, pReqSegBuf, pDescChain, true /*fFence*/);
+    int rc = virtioCoreR3QueueGet(pDevIns, &pThis->Virtio, EVENTQ_IDX, &pDescChain, true);
+    AssertRCReturn(rc, rc);
+
+    VIRTIOSCSI_EVENT_T Event;
+    Event.uEvent = uEventType;
+    Event.uReason = uReason;
+    Event.abVirtioLun[0] = 1;
+    Event.abVirtioLun[1] = uTarget;
+    Event.abVirtioLun[2] = (LUN0 >> 8) & 0x40;
+    Event.abVirtioLun[3] = LUN0 & 0xff;
+    Event.abVirtioLun[4] = 0;
+    Event.abVirtioLun[5] = 0;
+    Event.abVirtioLun[6] = 0;
+    Event.abVirtioLun[7] = 0;
+
+    RTSGSEG aReqSegs[1];
+    aReqSegs[0].pvSeg = &Event;
+    aReqSegs[0].cbSeg = sizeof(Event);
+
+    RTSGBUF ReqSgBuf;
+    RTSgBufInit(&ReqSgBuf, aReqSegs, RT_ELEMENTS(aReqSegs));
+
+    virtioCoreR3QueuePut(pDevIns, &pThis->Virtio, EVENTQ_IDX, &ReqSgBuf, pDescChain, true /*fFence*/);
     virtioCoreQueueSync(pDevIns, &pThis->Virtio, EVENTQ_IDX);
-
-    RTMemFree(paReqSegs[0].pvSeg);
-    RTMemFree(paReqSegs);
-    RTMemFree(pReqSegBuf);
 
     return VINF_SUCCESS;