Changeset 92878 in vbox

Timestamp:

Dec 13, 2021 9:47:14 AM (3 years ago)

Author:

vboxsync

Message:

Guest Control/Main: Resolved @todos: Renamed (plus removed) SESSIONOBJECTTYPE_ANONYMOUS -> SESSIONOBJECTTYPE_INVALID, use the internal guest object pointer directly to avoid a second lookup based on the object type.

Location:

trunk/src/VBox/Main

Files:

• include/GuestSessionImpl.h (modified) (2 diffs)
• src-client/GuestSessionImpl.cpp (modified) (2 diffs)

trunk/src/VBox/Main/include/GuestSessionImpl.h

@@ -242 +242 @@
     enum SESSIONOBJECTTYPE
     {
-        /** @todo r=bird: The tradition is to start at = 1, leaving 0 for invalid or
-         * unused to catch uninitialized data.  You do not number the other enum values.
-         * The 32BIT_HACK is only needed for externally visible stuff that might be used
-         * by with different compiler options.
-         *
-         * As noted elsewhere already, SESSIONOBJECTTYPE_ANONYMOUS isn't used
-         * anywhere can be removed till such time as it is needed.  Try to not think
-         * too far ahead but if you do, please leave some more useful clues that
-         * "Anonymous object" about the intent. */
-        /** Anonymous object. */
-        SESSIONOBJECTTYPE_ANONYMOUS  = 0,
+        /** Invalid session object type. */
+        SESSIONOBJECTTYPE_INVALID    = 0,
         /** Session object. */
         SESSIONOBJECTTYPE_SESSION    = 1,
@@ -260 +251 @@
         SESSIONOBJECTTYPE_FILE       = 3,
         /** Process object. */
-        SESSIONOBJECTTYPE_PROCESS    = 4,
-        /** The usual 32-bit hack. */
-        SESSIONOBJECTTYPE_32BIT_HACK = 0x7fffffff
+        SESSIONOBJECTTYPE_PROCESS    = 4
     };
 

trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp

@@ -1243 +1243 @@
     int rc = VERR_NOT_FOUND;
     const uint32_t idObject = VBOX_GUESTCTRL_CONTEXTID_GET_OBJECT(pCtxCb->uContextID);
-    SessionObjects::const_iterator itObjs = mData.mObjects.find(idObject);
-    if (itObjs != mData.mObjects.end())
+    SessionObjects::const_iterator itObj = mData.mObjects.find(idObject);
+    if (itObj != mData.mObjects.end())
     {
         /* Set protocol version so that pSvcCb can be interpreted right. */
         pCtxCb->uProtocol = mData.mProtocolVersion;
 
-        /** @todo r=bird: What is the meaning of this secondary lookup?  You've got the
-         * object pointer (except for SESSION where it's NULL because GuestSession
-         * doesn't inherit from GuestObject), and can just use the type to upcast it to
-         * grab a reference then call i_callbackDispatcher.
-         *
-         * Also, SESSIONOBJECTTYPE_ANONYMOUS is not used to remove it till needed.
-         * Don't think too far ahead, and when you do, please express why you think it
-         * is needed (the documentation of SESSIONOBJECTTYPE_ANONYMOUS is only repeating
-         * the obvious and not enlightening as to why you though it might come in
-         * useful).
-         */
-        switch (itObjs->second.enmType)
-        {
-            case SESSIONOBJECTTYPE_ANONYMOUS:
-                rc = VERR_NOT_SUPPORTED;
-                break;
-
+        switch (itObj->second.enmType)
+        {
+            /* Note: The session object is special, as it does not inherit from GuestObject we could call
+             *       its dispatcher for -- so treat this separately and call it directly. */
             case SESSIONOBJECTTYPE_SESSION:
             {
@@ -1275 +1262 @@
             case SESSIONOBJECTTYPE_DIRECTORY:
             {
-                SessionDirectories::const_iterator itDir = mData.mDirectories.find(idObject);
-                if (itDir != mData.mDirectories.end())
-                {
-                    ComObjPtr<GuestDirectory> pDirectory(itDir->second);
-                    Assert(!pDirectory.isNull());
-
-                    alock.release();
-
-                    rc = pDirectory->i_callbackDispatcher(pCtxCb, pSvcCb);
-                }
+                ComObjPtr<GuestDirectory> pObj((GuestDirectory *)itObj->second.pObject);
+                AssertReturn(!pObj.isNull(), VERR_INVALID_POINTER);
+
+                alock.release();
+
+                rc = pObj->i_callbackDispatcher(pCtxCb, pSvcCb);
                 break;
             }
             case SESSIONOBJECTTYPE_FILE:
             {
-                SessionFiles::const_iterator itFile = mData.mFiles.find(idObject);
-                if (itFile != mData.mFiles.end())
-                {
-                    ComObjPtr<GuestFile> pFile(itFile->second);
-                    Assert(!pFile.isNull());
-
-                    alock.release();
-
-                    rc = pFile->i_callbackDispatcher(pCtxCb, pSvcCb);
-                }
+                ComObjPtr<GuestFile> pObj((GuestFile *)itObj->second.pObject);
+                AssertReturn(!pObj.isNull(), VERR_INVALID_POINTER);
+
+                alock.release();
+
+                rc = pObj->i_callbackDispatcher(pCtxCb, pSvcCb);
                 break;
             }
             case SESSIONOBJECTTYPE_PROCESS:
             {
-                SessionProcesses::const_iterator itProc = mData.mProcesses.find(idObject);
-                if (itProc != mData.mProcesses.end())
-                {
-                    ComObjPtr<GuestProcess> pProcess(itProc->second);
-                    Assert(!pProcess.isNull());
-
-                    alock.release();
-
-                    rc = pProcess->i_callbackDispatcher(pCtxCb, pSvcCb);
-                }
+                ComObjPtr<GuestProcess> pObj((GuestProcess *)itObj->second.pObject);
+                AssertReturn(!pObj.isNull(), VERR_INVALID_POINTER);
+
+                alock.release();
+
+                rc = pObj->i_callbackDispatcher(pCtxCb, pSvcCb);
                 break;
             }
             default:
-                AssertMsgFailed(("%d\n", itObjs->second.enmType));
+                AssertMsgFailed(("%d\n", itObj->second.enmType));
                 rc = VERR_INTERNAL_ERROR_4;
                 break;