Changeset 82483 in vbox

Timestamp:

Dec 7, 2019 1:25:34 AM (5 years ago)

Author:

vboxsync

Message:

VBoxClipboardSvc: More save state cleanup. bugref:9437

Location:

trunk

Files:

• include/VBox/GuestHost/SharedClipboard.h (modified) (1 diff)
• src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-internal.h (modified) (2 diffs)
• src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp (modified) (6 diffs)

trunk/include/VBox/GuestHost/SharedClipboard.h

@@ -251 +251 @@
 /**
  * Shared Clipboard transfer source type.
+ * @note Part of saved state!
  */
 typedef enum SHCLSOURCE

trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-internal.h

@@ -91 +91 @@
 } SHCLCLIENTPODSTATE, *PSHCLCLIENTPODSTATE;
 
+/** @name SHCLCLIENTSTATE_FLAGS_XXX
+ * @note Part of saved state! */
 /** No Shared Clipboard client flags defined. */
 #define SHCLCLIENTSTATE_FLAGS_NONE              0
@@ -97 +99 @@
 /** Client has a guest write operation active. */
 #define SHCLCLIENTSTATE_FLAGS_WRITE_ACTIVE      RT_BIT(1)
+/** @} */
 
 /**

trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp

@@ -216 +216 @@
 #include <VBox/log.h>
 
-#include <VBox/AssertGuest.h>
 #include <VBox/GuestHost/clipboard-helper.h>
 #include <VBox/HostServices/Service.h>
@@ -222 +221 @@
 #include <VBox/HostServices/VBoxClipboardExt.h>
 
-#include <iprt/alloc.h>
+#include <VBox/AssertGuest.h>
+#include <VBox/err.h>
+#include <VBox/VMMDev.h>
+#include <VBox/vmm/ssm.h>
+
+#include <iprt/mem.h>
 #include <iprt/string.h>
 #include <iprt/assert.h>
 #include <iprt/critsect.h>
 #include <iprt/rand.h>
-
-#include <VBox/err.h>
-#include <VBox/vmm/ssm.h>
 
 #include "VBoxSharedClipboardSvc-internal.h"
@@ -2093 +2094 @@
 /**
  * SSM descriptor table for the SHCLCLIENTSTATE structure.
+ *
+ * @note Saving the session ID not necessary, as they're not persistent across
+ *       state save/restore.
  */
 static SSMFIELD const s_aShClSSMClientState[] =
 {
-    /** Note: Saving the session ID not necessary, as they're not persistent across state save/restore. */
     SSMFIELD_ENTRY(SHCLCLIENTSTATE, fGuestFeatures0),
     SSMFIELD_ENTRY(SHCLCLIENTSTATE, fGuestFeatures1),
@@ -2102 +2105 @@
     SSMFIELD_ENTRY(SHCLCLIENTSTATE, enmSource),
     SSMFIELD_ENTRY(SHCLCLIENTSTATE, fFlags),
+    SSMFIELD_ENTRY_TERM()
+};
+
+/**
+ * VBox 6.1 Beta 1 version of s_aShClSSMClientState (no flags).
+ */
+static SSMFIELD const s_aShClSSMClientState61B1[] =
+{
+    SSMFIELD_ENTRY(SHCLCLIENTSTATE, fGuestFeatures0),
+    SSMFIELD_ENTRY(SHCLCLIENTSTATE, fGuestFeatures1),
+    SSMFIELD_ENTRY(SHCLCLIENTSTATE, cbChunkSize),
+    SSMFIELD_ENTRY(SHCLCLIENTSTATE, enmSource),
     SSMFIELD_ENTRY_TERM()
 };
@@ -2266 +2281 @@
         if (lenOrVer >= VBOX_SHCL_SAVED_STATE_VER_6_1RC1)
         {
-            rc = SSMR3GetStructEx(pSSM, &pClient->State, sizeof(pClient->State), 0 /* fFlags */,
-                                  &s_aShClSSMClientState[0], NULL);
-            AssertRCReturn(rc, rc);
-
-            rc = SSMR3GetStructEx(pSSM, &pClient->State.POD, sizeof(pClient->State.POD), 0 /* fFlags */,
-                                  &s_aShClSSMClientPODState[0], NULL);
-            AssertRCReturn(rc, rc);
-        }
-        else /** @todo Remove this block after 6.1 RC; don't annoy team members with broken saved states. */
-        {
-            rc = SSMR3Skip(pSSM, sizeof(uint32_t));         /* Begin marker */
-            AssertRC(rc);
-
-            rc = SSMR3GetU64(pSSM, &pClient->State.fGuestFeatures0);
-            AssertRC(rc);
-
-            rc = SSMR3GetU64(pSSM, &pClient->State.fGuestFeatures1);
-            AssertRC(rc);
-
-            rc = SSMR3GetU32(pSSM, &pClient->State.cbChunkSize);
-            AssertRC(rc);
-
-            rc = SSMR3GetU32(pSSM, (uint32_t *)&pClient->State.enmSource);
-            AssertRC(rc);
-
-            rc = SSMR3Skip(pSSM, sizeof(uint32_t));         /* End marker */
-            AssertRC(rc);
-        }
-
+            SSMR3GetStructEx(pSSM, &pClient->State, sizeof(pClient->State), 0 /* fFlags */, &s_aShClSSMClientState[0], NULL);
+            SSMR3GetStructEx(pSSM, &pClient->State.POD, sizeof(pClient->State.POD), 0 /* fFlags */,
+                             &s_aShClSSMClientPODState[0], NULL);
+        }
+        else
+            SSMR3GetStructEx(pSSM, &pClient->State, sizeof(pClient->State), 0 /* fFlags */, &s_aShClSSMClientState61B1[0], NULL);
         rc = SSMR3GetStructEx(pSSM, &pClient->State.Transfers, sizeof(pClient->State.Transfers), 0 /* fFlags */,
                               &s_aShClSSMClientTransferState[0], NULL);
@@ -2300 +2292 @@
 
         /* Load the client's internal message queue. */
-        uint64_t cMsg;
-        rc = SSMR3GetU64(pSSM, &cMsg);
+        uint64_t cMsgs;
+        rc = SSMR3GetU64(pSSM, &cMsgs);
         AssertRCReturn(rc, rc);
-
-        for (uint64_t i = 0; i < cMsg; i++)
+        AssertLogRelMsgReturn(cMsgs < _16K, ("Too many messages: %u (%x)\n", cMsgs, cMsgs), VERR_SSM_DATA_UNIT_FORMAT_CHANGED);
+
+        for (uint64_t i = 0; i < cMsgs; i++)
         {
             PSHCLCLIENTMSG pMsg = (PSHCLCLIENTMSG)RTMemAlloc(sizeof(SHCLCLIENTMSG));
-            AssertPtrReturn(pMsg, VERR_NO_MEMORY);
-
-            rc = SSMR3GetStructEx(pSSM, pMsg, sizeof(SHCLCLIENTMSG), 0 /*fFlags*/, &s_aShClSSMClientMsgHdr[0], NULL);
-            AssertRCReturn(rc, rc);
-
+            AssertReturn(pMsg, VERR_NO_MEMORY);
+
+            SSMR3GetStructEx(pSSM, pMsg, sizeof(SHCLCLIENTMSG), 0 /*fFlags*/, &s_aShClSSMClientMsgHdr[0], NULL);
             rc = SSMR3GetStructEx(pSSM, &pMsg->Ctx, sizeof(SHCLMSGCTX), 0 /*fFlags*/, &s_aShClSSMClientMsgCtx[0], NULL);
-            AssertRCReturn(rc, rc);
+            AssertRCReturnStmt(rc, RTMemFree(pMsg), rc);
+
+            AssertLogRelMsgReturnStmt(pMsg->cParms <= VMMDEV_MAX_HGCM_PARMS,
+                                      ("Too many HGCM message parameters: %u (%#x)\n", pMsg->cParms, pMsg->cParms),
+                                      RTMemFree(pMsg),
+                                      VERR_SSM_DATA_UNIT_FORMAT_CHANGED);
 
             pMsg->paParms = (PVBOXHGCMSVCPARM)RTMemAllocZ(sizeof(VBOXHGCMSVCPARM) * pMsg->cParms);
-            AssertPtrReturn(pMsg->paParms, VERR_NO_MEMORY);
+            AssertReturnStmt(pMsg->paParms, RTMemFree(pMsg), VERR_NO_MEMORY);
 
             for (uint32_t p = 0; p < pMsg->cParms; p++)
             {
                 rc = HGCMSvcSSMR3Get(&pMsg->paParms[p], pSSM);
-                AssertRCReturn(rc, rc);
+                AssertRCReturn(rc, rc); /* we'll leak stuff here, oh well... */
             }