Changeset 14213 in vbox for trunk/src/VBox

Timestamp:

Nov 14, 2008 2:45:40 PM (16 years ago)

Author:

vboxsync

Message:

HostServices/GuestProperties: some clean up

Location:

trunk/src/VBox/HostServices/GuestProperties

Files:

• service.cpp (modified) (17 diffs)
• testcase/tstGuestPropSvc.cpp (modified) (3 diffs)

trunk/src/VBox/HostServices/GuestProperties/service.cpp

@@ -64 +64 @@
 
 /**
+ * Structure for holding a property
+ */
+struct Property
+{
+    /** The name of the property */
+    std::string mName;
+    /** The property value */
+    std::string mValue;
+    /** The timestamp of the property */
+    uint64_t mTimestamp;
+    /** The property flags */
+    uint32_t mFlags;
+
+    /** Default constructor */
+    Property() : mTimestamp(0), mFlags(NILFLAG) {}
+    /** Constructor with const char * */
+    Property(const char *pcszName, const char *pcszValue,
+             uint64_t u64Timestamp, uint32_t u32Flags)
+        : mName(pcszName), mValue(pcszValue), mTimestamp(u64Timestamp),
+          mFlags(u32Flags) {}
+    /** Constructor with std::string */
+    Property(std::string name, std::string value, uint64_t u64Timestamp,
+             uint32_t u32Flags)
+        : mName(name), mValue(value), mTimestamp(u64Timestamp),
+          mFlags(u32Flags) {}
+
+    /** Does the property name match one of a set of patterns? */
+    bool Matches(const char *pszPatterns) const
+    {
+        return (   pszPatterns[0] == '\0'  /* match all */
+                || RTStrSimplePatternMultiMatch(pszPatterns, RTSTR_MAX,
+                                                mName.c_str(), RTSTR_MAX,
+                                                NULL)
+               );
+    }
+
+    /** Are two properties equal? */
+    bool operator== (const Property &prop)
+    {
+        return (   mName == prop.mName
+                && mValue == prop.mValue
+                && mTimestamp == prop.mTimestamp
+                && mFlags == prop.mFlags
+               );
+    }
+
+    /* Is the property nil? */
+    bool isNull()
+    {
+        return mName.empty();
+    }
+};
+/** The properties list type */
+typedef std::list <Property> PropertyList;
+
+/**
+ * Structure for holding an uncompleted guest call
+ */
+struct GuestCall
+{
+    /** The call handle */
+    VBOXHGCMCALLHANDLE mHandle;
+    /** The function that was requested */
+    uint32_t mFunction;
+    /** The call parameters */
+    VBOXHGCMSVCPARM *mParms;
+    /** The default return value, used for passing warnings */
+    int mRc;
+
+    /** The standard constructor */
+    GuestCall() : mFunction(0) {}
+    /** The normal contructor */
+    GuestCall(VBOXHGCMCALLHANDLE aHandle, uint32_t aFunction,
+              VBOXHGCMSVCPARM aParms[], int aRc)
+              : mHandle(aHandle), mFunction(aFunction), mParms(aParms),
+                mRc(aRc) {}
+};
+/** The guest call list type */
+typedef std::list <GuestCall> CallList;
+
+/**
  * Class containing the shared information service functionality.
  */
@@ -73 +154 @@
     /** HGCM helper functions. */
     PVBOXHGCMSVCHELPERS mpHelpers;
-    /** Structure for holding a property */
-    struct Property
-    {
-        /** The name of the property */
-        std::string mName;
-        /** The property value */
-        std::string mValue;
-        /** The timestamp of the property */
-        uint64_t mTimestamp;
-        /** The property flags */
-        uint32_t mFlags;
-
-        /** Default constructor */
-        Property() : mName(""), mValue(""), mTimestamp(0), mFlags(NILFLAG) {}
-        /** Constructor with const char * */
-        Property(const char *pcszName, const char *pcszValue,
-                 uint64_t u64Timestamp, uint32_t u32Flags)
-            : mName(pcszName), mValue(pcszValue), mTimestamp(u64Timestamp),
-              mFlags(u32Flags) {}
-        /** Constructor with std::string */
-        Property(std::string name, std::string value, uint64_t u64Timestamp,
-                 uint32_t u32Flags)
-            : mName(name), mValue(value), mTimestamp(u64Timestamp),
-              mFlags(u32Flags) {}
-    };
-    /** The properties list type */
-    typedef std::list <Property> PropertyList;
     /** The property list */
     PropertyList mProperties;
     /** The list of property changes for guest notifications */
     PropertyList mGuestNotifications;
-    /** Structure for holding an uncompleted guest call */
-    struct GuestCall
-    {
-        /** The call handle */
-        VBOXHGCMCALLHANDLE mHandle;
-        /** The function that was requested */
-        uint32_t mFunction;
-        /** The call parameters */
-        VBOXHGCMSVCPARM *mParms;
-        /** The default return value, used for passing warnings */
-        int mRc;
-
-        /** The standard constructor */
-        GuestCall() : mFunction(0) {}
-        /** The normal contructor */
-        GuestCall(VBOXHGCMCALLHANDLE aHandle, uint32_t aFunction,
-                  VBOXHGCMSVCPARM aParms[], int aRc)
-                  : mHandle(aHandle), mFunction(aFunction), mParms(aParms),
-                    mRc(aRc) {}
-    };
-    typedef std::list <GuestCall> CallList;
     /** The list of outstanding guest notification calls */
     CallList mGuestWaiters;
@@ -140 +173 @@
     void *mpvHostData;
 
+    /**
+     * Get the next property change notification from the queue of saved
+     * notification based on the timestamp of the last notification seen.
+     * Notifications will only be reported if the property name matches the
+     * pattern given.
+     *
+     * @returns iprt status value
+     * @returns VWRN_NOT_FOUND if the last notification was not found in the queue
+     * @param   pszPatterns   the patterns to match the property name against
+     * @param   u64Timestamp  the timestamp of the last notification
+     * @param   pProp         where to return the property found.  If none is
+     *                        found this will be set to nil.
+     * @thread  HGCM
+     */
+    int getOldNotification(const char *pszPatterns, uint64_t u64Timestamp,
+                           Property *pProp)
+    {
+        AssertPtrReturn(pszPatterns, VERR_INVALID_POINTER);
+        /* Zero means wait for a new notification. */
+        AssertReturn(u64Timestamp != 0, VERR_INVALID_PARAMETER);
+        AssertPtrReturn(pProp, VERR_INVALID_POINTER);
+        int rc = getOldNotificationInternal(pszPatterns, u64Timestamp, pProp);
+#ifdef DEBUG
+        /* 
+         * ENSURE that pProp is the first event in the notification queue that:
+         *  - Appears later than u64Timestamp
+         *  - Matches the pszPatterns
+         */
+        PropertyList::const_iterator it = mGuestNotifications.begin();
+        for (;    it != mGuestNotifications.end()
+               && it->mTimestamp != u64Timestamp; ++it) {}
+        if (it == mGuestNotifications.end())  /* Not found */
+            it = mGuestNotifications.begin();
+        else
+            ++it;  /* Next event */
+        for (;    it != mGuestNotifications.end()
+               && it->mTimestamp != pProp->mTimestamp; ++it)
+            Assert(!it->Matches(pszPatterns));
+        if (pProp->mTimestamp != 0)
+        {
+            Assert(*pProp == *it);
+            Assert(pProp->Matches(pszPatterns));
+        }
+#endif
+        return rc;
+    }
+
 public:
     explicit Service(PVBOXHGCMSVCHELPERS pHelpers)
@@ -238 +318 @@
     int getNotification(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms,
                         VBOXHGCMSVCPARM paParms[]);
-    int getOldNotification(const char *pszPattern, uint64_t u64Timestamp,
-                           Property *pProp);
+    int getOldNotificationInternal(const char *pszPattern,
+                                   uint64_t u64Timestamp, Property *pProp);
     int getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop);
     void doNotifications(const char *pszProperty, uint64_t u64Timestamp);
@@ -704 +784 @@
      * First repack the patterns into the format expected by RTStrSimplePatternMatch()
      */
-    bool matchAll = false;
     char pszPatterns[MAX_PATTERN_LEN];
-    if (   (NULL == pcchPatterns)
-        || (cchPatterns < 2)  /* An empty pattern string means match all */
-       )
-        matchAll = true;
+    if (NULL == pcchPatterns)
+        pszPatterns[0] = '\0';
     else
     {
@@ -728 +805 @@
          RT_SUCCESS(rc) && (it != mProperties.end()); ++it)
     {
-        if (   matchAll
-            || RTStrSimplePatternMultiMatch(pszPatterns, RTSTR_MAX,
-                                            it->mName.c_str(), RTSTR_MAX, NULL)
-           )
+        if (it->Matches(pszPatterns))
         {
             char szFlags[MAX_FLAGS_LEN];
@@ -768 +842 @@
 }
 
-/**
- * Get the next property change notification from the queue of saved
- * notification based on the timestamp of the last notification seen.
- * Notifications will only be reported if the property name matches the
- * pattern given.
- *
- * @returns iprt status value
- * @returns VWRN_NOT_FOUND if the last notification was not found in the queue
- * @param   pszPatterns   the patterns to match the property name against
- * @param   u64Timestamp  the timestamp of the last notification
- * @param   pProp         where to return the property found.  If none is
- *                        found this will be set to nil.
- * @thread  HGCM
- */
-int Service::getOldNotification(const char *pszPatterns, uint64_t u64Timestamp,
-                                Property *pProp)
-{
-    AssertPtrReturn(pszPatterns, VERR_INVALID_POINTER);
-    AssertReturn(u64Timestamp != 0, VERR_INVALID_PARAMETER);  /* Zero means wait for a new notification. */
-    AssertPtrReturn(pProp, VERR_INVALID_POINTER);
+/** Helper query used by getOldNotification */
+int Service::getOldNotificationInternal(const char *pszPatterns,
+                                        uint64_t u64Timestamp,
+                                        Property *pProp)
+{
     int rc = VINF_SUCCESS;
     bool warn = false;
@@ -802 +861 @@
      * member conveniently points to the following element. */
     PropertyList::iterator base = it.base();
-    for (;    pszPatterns[0] != '\0'
-           && !RTStrSimplePatternMultiMatch(pszPatterns, RTSTR_MAX,
-                                            base->mName.c_str(), RTSTR_MAX,
-                                            NULL)
-           && base != mGuestNotifications.end(); ++base) {}
+    for (; !base->Matches(pszPatterns) && base != mGuestNotifications.end();
+         ++base) {}
     if (RT_SUCCESS(rc) && base != mGuestNotifications.end())
         *pProp = *base;
@@ -816 +872 @@
 }
 
+/** Helper query used by getNotification */
 int Service::getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop)
 {
@@ -884 +941 @@
 
     /*
-     * Find the change to notify of.
+     * If no timestamp was supplied or no notification was found in the queue
+     * of old notifications, enqueue the request in the waiting queue.
      */
     Property prop;
     if (RT_SUCCESS(rc) && u64Timestamp != 0)
         rc = getOldNotification(pszPatterns, u64Timestamp, &prop);
-    if (   (RT_SUCCESS(rc) && u64Timestamp == 0)
-        || (RT_SUCCESS(rc) && prop.mName.size() == 1)  /* Empty name -> not found */
-       )
+    if (RT_SUCCESS(rc) && prop.isNull())
     {
         mGuestWaiters.push_back(GuestCall(callHandle, GET_NOTIFICATION,
@@ -897 +953 @@
         rc = VINF_HGCM_ASYNC_EXECUTE;
     }
+    /*
+     * Otherwise reply at once with the enqueued notification we found.
+     */
     else
     {
@@ -918 +977 @@
 void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
 {
-    char *pszName = NULL, *pszValue = NULL, *pszFlags = NULL;
     int rc = VINF_SUCCESS;
 
@@ -928 +986 @@
 
     /*
-     * Try to find the property.
+     * Try to find the property.  Create a change event if we find it and a
+     * delete event if we do not.
      */
     Property prop;
+    prop.mName = pszProperty;
+    prop.mTimestamp = u64Timestamp;
+    /* prop is currently a delete event for pszProperty */
     bool found = false;
     if (RT_SUCCESS(rc))
@@ -938 +1000 @@
             {
                 found = true;
-                prop = *it;
+                /* Make prop into a change event. */
+                prop.mValue = it->mValue;
+                prop.mFlags = it->mFlags;
             }
 
-    /*
-     * First case: if the property exists then send its current value
-     */
+
+    /* Release waiters if applicable and add the event to the queue for
+     * guest notifications */
+    if (RT_SUCCESS(rc))
+    {
+        try
+        {
+            for (CallList::iterator it = mGuestWaiters.begin();
+                 it != mGuestWaiters.end(); ++it)
+            {
+                const char *pszPatterns;
+                uint32_t cchPatterns;
+                it->mParms[0].getPointer((void **) &pszPatterns, &cchPatterns);
+                if (prop.Matches(pszPatterns))
+                {
+                    GuestCall call = mGuestWaiters.back();
+                    int rc2 = getNotificationWriteOut(call.mParms, prop);
+                    if (RT_SUCCESS(rc2))
+                        rc2 = call.mRc;
+                    mpHelpers->pfnCallComplete (call.mHandle, rc2);
+                    it = mGuestWaiters.erase(it);
+                }
+            }
+            mGuestNotifications.push_back(prop);
+        }
+        catch (std::bad_alloc)
+        {
+            rc = VERR_NO_MEMORY;
+        }
+    }
+    if (mGuestNotifications.size() > MAX_GUEST_NOTIFICATIONS)
+        mGuestNotifications.pop_front();
+
+#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
+    /*
+     * Host notifications - first case: if the property exists then send its
+     * current value
+     */
+    char *pszName = NULL, *pszValue = NULL, *pszFlags = NULL;
+
     if (found && mpfnHostCallback != NULL)
     {
-#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
         char szFlags[MAX_FLAGS_LEN];
         /* Send out a host notification */
@@ -962 +1062 @@
                              (uint32_t) RT_HIDWORD(u64Timestamp),
                              (uint32_t) RT_LODWORD(u64Timestamp), pszFlags);
-#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
-    }
-    if (found)
-    {
-        /* Release waiters if applicable and add the change to the queue for
-         * guest notifications */
-        if (RT_SUCCESS(rc))
-        {
-            try
-            {
-                for (CallList::iterator it = mGuestWaiters.begin();
-                     it != mGuestWaiters.end(); ++it)
-                {
-                    const char *pszPatterns;
-                    uint32_t cchPatterns;
-                    it->mParms[0].getPointer((void **) &pszPatterns, &cchPatterns);
-                    if (   pszPatterns[0] == '\0'
-                        || RTStrSimplePatternMultiMatch(pszPatterns, RTSTR_MAX,
-                                                        pszProperty, RTSTR_MAX,
-                                                        NULL))
-                    {
-                        GuestCall call = mGuestWaiters.back();
-                        int rc2 = getNotificationWriteOut(call.mParms, prop);
-                        if (RT_SUCCESS(rc2))
-                            rc2 = call.mRc;
-                        mpHelpers->pfnCallComplete (call.mHandle, rc2);
-                        it = mGuestWaiters.erase(it);
-                    }
-                }
-                mGuestNotifications.push_back(prop);
-            }
-            catch (std::bad_alloc)
-            {
-                rc = VERR_NO_MEMORY;
-            }
-        }
-    }
-
-    /*
-     * Second case: if the property does not exist then send the host an empty
-     * value
+    }
+
+    /*
+     * Host notifications - second case: if the property does not exist then
+     * send the host an empty value
      */
     if (!found && mpfnHostCallback != NULL)
     {
-#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
         /* Send out a host notification */
         rc = RTStrDupEx(&pszName, pszProperty);
@@ -1015 +1078 @@
                              (uint32_t) RT_HIDWORD(u64Timestamp),
                              (uint32_t) RT_LODWORD(u64Timestamp), NULL);
-#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
-    }
-    if (!found)
-    {
-        /* Add the change to the queue for guest notifications */
-        if (RT_SUCCESS(rc))
-        {
-            try
-            {
-                mGuestNotifications.push_back(Property(pszProperty, "",
-                                                       u64Timestamp, NILFLAG)
-                                             );
-            }
-            catch (std::bad_alloc)
-            {
-                rc = VERR_NO_MEMORY;
-            }
-        }
-    }
-    if (mGuestNotifications.size() > MAX_GUEST_NOTIFICATIONS)
-        mGuestNotifications.pop_front();
+    }
     if (RT_FAILURE(rc)) /* clean up if we failed somewhere */
     {
@@ -1042 +1085 @@
         RTStrFree(pszFlags);
     }
+#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
 }
 

trunk/src/VBox/HostServices/GuestProperties/testcase/tstGuestPropSvc.cpp

@@ -460 +460 @@
         strncat(szValue, setProperties[i].pcszValue, sizeof(szValue));
         strncat(szFlags, setProperties[i].pcszFlags, sizeof(szFlags));
-        paParms[0].setPointer (szName, strlen(szName) + 1);
-        paParms[1].setPointer (szValue, strlen(szValue) + 1);
-        paParms[2].setPointer (szFlags, strlen(szFlags) + 1);
+        paParms[0].setPointer (szName, (uint32_t)strlen(szName) + 1);
+        paParms[1].setPointer (szValue, (uint32_t)strlen(szValue) + 1);
+        paParms[2].setPointer (szFlags, (uint32_t)strlen(szFlags) + 1);
         if (setProperties[i].isHost)
             callHandle.rc = pTable->pfnHostCall(pTable->pvService, command,
@@ -532 +532 @@
         char szName[MAX_NAME_LEN] = "";
         strncat(szName, delProperties[i].pcszName, sizeof(szName));
-        paParms[0].setPointer (szName, strlen(szName) + 1);
+        paParms[0].setPointer (szName, (uint32_t)strlen(szName) + 1);
         if (delProperties[i].isHost)
             callHandle.rc = pTable->pfnHostCall(pTable->pvService, command,
@@ -609 +609 @@
                         rc = VERR_INTERNAL_ERROR);
         strncat(szName, getProperties[i].pcszName, sizeof(szName));
-        paParms[0].setPointer (szName, strlen(szName) + 1);
+        paParms[0].setPointer (szName, (uint32_t)strlen(szName) + 1);
         paParms[1].setPointer (szBuffer, sizeof(szBuffer));
         rc2 = pTable->pfnHostCall(pTable->pvService, GET_PROP_HOST, 4,