Changeset 75861 in vbox for trunk/src/VBox/Main/include

Timestamp:

Dec 2, 2018 12:26:36 AM (6 years ago)

Author:

vboxsync

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

127065

Message:

Main/Guest: Added a few codereview comments, mainly in the guest base class area. Replaced the object ID management in GuestSession with a classic allocation bitmap approach as that's much less memory intensive than 16 byte structs in a insert-only std::map and an additional map tracking free 32-bit IDs after we run out. bugref:9313

Location:

trunk/src/VBox/Main/include

Files:

• GuestCtrlImplPrivate.h (modified) (2 diffs)
• GuestSessionImpl.h (modified) (8 diffs)

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

@@ -1078 +1078 @@
     int                              GetGuestError(void) const { return mGuestRc; }
     int                              SignalExternal(IEvent *pEvent);
-    const GuestEventTypes            Types(void) { return mEventTypes; }
+    const GuestEventTypes           &Types(void) { return mEventTypes; }
     size_t                           TypeCount(void) { return mEventTypes.size(); }
 
@@ -1140 +1140 @@
      *  for HGCM (VMMDev) communication. */
     Console                 *mConsole;
-    /** The next upcoming context ID for this object. */
+    /**  The next context ID counter component for this object. */
     uint32_t                 mNextContextID;
     /** Local listener for handling the waiting events

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

@@ -29 +29 @@
 #include "GuestSessionImplTasks.h"
 
+#include <iprt/asm.h> /** @todo r=bird: Needed for ASMBitSet() in GuestSession::Data constructor.  Removed when
+                       *        that is moved into the class implementation file as it should be. */
 #include <deque>
 
@@ -36 +38 @@
  * Guest session implementation.
  */
-class ATL_NO_VTABLE GuestSession :
-    public GuestSessionWrap,
-    public GuestBase
+class ATL_NO_VTABLE GuestSession
+    : public GuestSessionWrap
+    , public GuestBase
 {
 public:
@@ -245 +247 @@
     struct SessionObject
     {
-        /** Creation timestamp (in ms). */
-        uint64_t          tsCreatedMs;
+        /** Creation timestamp (in ms).
+         * @note not used by anyone at the moment.  */
+        uint64_t          msBirth;
         /** The object type. */
         SESSIONOBJECTTYPE enmType;
@@ -305 +308 @@
     Guest                  *i_getParent(void) { return mParent; }
     uint32_t                i_getProtocolVersion(void) { return mData.mProtocolVersion; }
-    int                     i_objectRegister(SESSIONOBJECTTYPE enmType, uint32_t *puObjectID);
-    int                     i_objectRegisterEx(SESSIONOBJECTTYPE enmType, uint32_t fFlags, uint32_t *puObjectID);
+    int                     i_objectRegister(SESSIONOBJECTTYPE enmType, uint32_t *pidObject);
     int                     i_objectUnregister(uint32_t uObjectID);
     int                     i_pathRename(const Utf8Str &strSource, const Utf8Str &strDest, uint32_t uFlags, int *pGuestRc);
@@ -341 +343 @@
     const ComObjPtr<EventSource>    mEventSource;
 
+    /** @todo r=bird: One of the core points of the DATA sub-structures in Main is
+     * hinding implementation details and stuff that requires including iprt/asm.h.
+     * The way it's used here totally defeats that purpose.  You need to make it
+     * a pointer to a anynmous Data struct and define that structure in
+     * GuestSessionImpl.cpp and allocate it in the Init() function.
+     */
     struct Data
     {
@@ -367 +375 @@
         /** Map of registered session objects (files, directories, ...). */
         SessionObjects              mObjects;
-        /** Queue of object IDs which are not used anymore (free list).
-         *  Acts as a "free list" for the mObjects map. */
-        SessionObjectsFree          mObjectsFree;
         /** Guest control protocol version to be used.
          *  Guest Additions < VBox 4.3 have version 1,
@@ -379 +384 @@
          *  returned from the guest side. */
         int                         mRC;
+        /** Object ID allocation bitmap; clear bits are free, set bits are busy. */
+        uint64_t                    bmObjectIds[VBOX_GUESTCTRL_MAX_OBJECTS / sizeof(uint64_t) / 8];
 
         Data(void)
             : mpBaseEnvironment(NULL)
-        { }
+        {
+            RT_ZERO(bmObjectIds);
+            ASMBitSet(&bmObjectIds, VBOX_GUESTCTRL_MAX_OBJECTS - 1);    /* Reserved for the session itself? */
+            ASMBitSet(&bmObjectIds, 0);                                 /* Let's reserve this too. */
+        }
         Data(const Data &rThat)
             : mCredentials(rThat.mCredentials)
@@ -393 +404 @@
             , mProcesses(rThat.mProcesses)
             , mObjects(rThat.mObjects)
-            , mObjectsFree(rThat.mObjectsFree)
             , mProtocolVersion(rThat.mProtocolVersion)
             , mTimeout(rThat.mTimeout)
             , mRC(rThat.mRC)
-        { }
+        {
+            memcpy(&bmObjectIds, &rThat.bmObjectIds, sizeof(bmObjectIds));
+        }
         ~Data(void)
         {