Changeset 44155 in vbox for trunk/src/VBox/HostServices/GuestProperties

Timestamp:

Dec 18, 2012 8:13:37 PM (12 years ago)

Author:

vboxsync

Message:

GuestProperties/service: Misc. sanity checking.

File:

• trunk/src/VBox/HostServices/GuestProperties/service.cpp (modified) (20 diffs)

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

@@ -139 +139 @@
     /** The function that was requested */
     uint32_t mFunction;
+    /** Number of call parameters. */
+    uint32_t mParmsCnt;
     /** The call parameters */
     VBOXHGCMSVCPARM *mParms;
@@ -145 +147 @@
 
     /** The standard constructor */
-    GuestCall() : u32ClientId(0), mFunction(0) {}
+    GuestCall(void) : u32ClientId(0), mFunction(0), mParmsCnt(0) {}
     /** The normal constructor */
     GuestCall(uint32_t aClientId, VBOXHGCMCALLHANDLE aHandle, uint32_t aFunction,
-              VBOXHGCMSVCPARM aParms[], int aRc)
-              : u32ClientId(aClientId), mHandle(aHandle), mFunction(aFunction), mParms(aParms),
-                mRc(aRc) {}
+              uint32_t aParmsCnt, VBOXHGCMSVCPARM aParms[], int aRc)
+              : u32ClientId(aClientId), mHandle(aHandle), mFunction(aFunction),
+                mParmsCnt(aParmsCnt), mParms(aParms), mRc(aRc) {}
 };
 /** The guest call list type */
@@ -171 +173 @@
     /** The number of properties. */
     unsigned mcProperties;
-    /** The list of property changes for guest notifications */
+    /** The list of property changes for guest notifications;
+     *  only used for timestamp tracking in notifications at the moment */
     PropertyList mGuestNotifications;
     /** The list of outstanding guest notification calls */
@@ -376 +379 @@
     int getOldNotificationInternal(const char *pszPattern,
                                    uint64_t u64Timestamp, Property *pProp);
-    int getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop);
-    void doNotifications(const char *pszProperty, uint64_t u64Timestamp);
+    int getNotificationWriteOut(uint32_t cParms, VBOXHGCMSVCPARM paParms[], Property prop);
+    int doNotifications(const char *pszProperty, uint64_t u64Timestamp);
     int notifyHost(const char *pszName, const char *pszValue,
                    uint64_t u64Timestamp, const char *pszFlags);
@@ -702 +705 @@
         else if (mcProperties < MAX_PROPS)
         {
-            /* Create a new string space record. */
-            pProp = new Property(pcszName, pcszValue, u64TimeNano, fFlags);
-            if (pProp)
+            try
             {
+                /* Create a new string space record. */
+                pProp = new Property(pcszName, pcszValue, u64TimeNano, fFlags);
+                AssertPtr(pProp);
+
                 if (RTStrSpaceInsert(&mhProperties, &pProp->mStrCore))
                     mcProperties++;
@@ -712 +717 @@
                     AssertFailed();
                     delete pProp;
-                    rc = VERR_INTERNAL_ERROR_3;
+
+                    rc = VERR_ALREADY_EXISTS;
                 }
             }
-            else
+            catch (std::bad_alloc)
+            {
                 rc = VERR_NO_MEMORY;
+            }
         }
         else
@@ -722 +730 @@
 
         /*
-         * Send a notification to the host and return.
+         * Send a notification to the guest and host and return.
          */
         // if (isGuest)  /* Notify the host even for properties that the host
         //                * changed.  Less efficient, but ensures consistency. */
-            doNotifications(pcszName, u64TimeNano);
-        Log2(("Set string %s, rc=%Rrc, value=%s\n", pcszName, rc, pcszValue));
-    }
-
-    LogFlowThisFunc(("rc = %Rrc (%s = %s)\n", rc, pcszName, pcszValue));
+        int rc2 = doNotifications(pcszName, u64TimeNano);
+        if (RT_SUCCESS(rc))
+            rc = rc2;
+    }
+
+    LogFlowThisFunc(("%s=%s, rc=%Rrc\n", pcszName, pcszValue, rc));
     return rc;
 }
@@ -764 +773 @@
     if (RT_FAILURE(rc))
     {
-        LogFlowThisFunc(("rc = %Rrc\n", rc));
+        LogFlowThisFunc(("rc=%Rrc\n", rc));
         return rc;
     }
@@ -788 +797 @@
         // if (isGuest)  /* Notify the host even for properties that the host
         //                * changed.  Less efficient, but ensures consistency. */
-            doNotifications(pcszName, u64Timestamp);
-    }
-
-    LogFlowThisFunc(("rc = %Rrc (%s)\n", rc, pcszName));
+        int rc2 = doNotifications(pcszName, u64Timestamp);
+        if (RT_SUCCESS(rc))
+            rc = rc2;
+    }
+
+    LogFlowThisFunc(("%s: rc=%Rrc\n", pcszName, rc));
     return rc;
 }
@@ -968 +979 @@
 
 /** Helper query used by getNotification */
-int Service::getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop)
-{
-    int rc = VINF_SUCCESS;
+int Service::getNotificationWriteOut(uint32_t cParms, VBOXHGCMSVCPARM paParms[], Property prop)
+{
+    AssertReturn(cParms == 4, VERR_INVALID_PARAMETER); /* Basic sanity checking. */
+
     /* Format the data to write to the buffer. */
     std::string buffer;
@@ -976 +988 @@
     char *pchBuf;
     uint32_t cbBuf;
-    rc = paParms[2].getBuffer((void **)&pchBuf, &cbBuf);
+
+    int rc = paParms[2].getBuffer((void **)&pchBuf, &cbBuf);
     if (RT_SUCCESS(rc))
     {
@@ -1015 +1028 @@
  * @throws  can throw std::bad_alloc
  */
-int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms,
-                             VBOXHGCMSVCPARM paParms[])
+int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle,
+                             uint32_t cParms, VBOXHGCMSVCPARM paParms[])
 {
     int rc = VINF_SUCCESS;
@@ -1035 +1048 @@
        )
         rc = VERR_INVALID_PARAMETER;
+
     if (RT_SUCCESS(rc))
-        LogFlow(("    pszPatterns=%s, u64Timestamp=%llu\n", pszPatterns,
-                 u64Timestamp));
+        LogFlow(("pszPatterns=%s, u64Timestamp=%llu\n", pszPatterns, u64Timestamp));
 
     /*
@@ -1046 +1059 @@
     if (RT_SUCCESS(rc) && u64Timestamp != 0)
         rc = getOldNotification(pszPatterns, u64Timestamp, &prop);
-    if (RT_SUCCESS(rc) && prop.isNull())
-    {
+    if (RT_SUCCESS(rc))
+    {
+        if (prop.isNull())
+        {
+            /*
+             * Check if the client already had the same request.
+             * Complete the old request with an error in this case.
+             * Protection against clients, which cancel and resubmits requests.
+             */
+            CallList::iterator it = mGuestWaiters.begin();
+            while (it != mGuestWaiters.end())
+            {
+                const char *pszPatternsExisting;
+                uint32_t cchPatternsExisting;
+                int rc3 = it->mParms[0].getString(&pszPatternsExisting, &cchPatternsExisting);
+
+                if (   RT_SUCCESS(rc3)
+                    && u32ClientId == it->u32ClientId
+                    && RTStrCmp(pszPatterns, pszPatternsExisting) == 0)
+                {
+                    /* Complete the old request. */
+                    mpHelpers->pfnCallComplete(it->mHandle, VERR_INTERRUPTED);
+                    it = mGuestWaiters.erase(it);
+                }
+                else
+                    ++it;
+            }
+
+            mGuestWaiters.push_back(GuestCall(u32ClientId, callHandle, GET_NOTIFICATION,
+                                              cParms, paParms, rc));
+            rc = VINF_HGCM_ASYNC_EXECUTE;
+        }
         /*
-         * Check if the client already had the same request.
-         * Complete the old request with an error in this case.
-         * Protection against clients, which cancel and resubmits requests.
+         * Otherwise reply at once with the enqueued notification we found.
          */
-        CallList::iterator it = mGuestWaiters.begin();
-        while (it != mGuestWaiters.end())
-        {
-            const char *pszPatternsExisting;
-            uint32_t cchPatternsExisting;
-            int rc3 = it->mParms[0].getString(&pszPatternsExisting, &cchPatternsExisting);
-
-            if (   RT_SUCCESS(rc3)
-                && u32ClientId == it->u32ClientId
-                && RTStrCmp(pszPatterns, pszPatternsExisting) == 0)
-            {
-                /* Complete the old request. */
-                mpHelpers->pfnCallComplete(it->mHandle, VERR_INTERRUPTED);
-                it = mGuestWaiters.erase(it);
-            }
-            else
-                ++it;
-        }
-
-        mGuestWaiters.push_back(GuestCall(u32ClientId, callHandle, GET_NOTIFICATION,
-                                          paParms, rc));
-        rc = VINF_HGCM_ASYNC_EXECUTE;
-    }
-    /*
-     * Otherwise reply at once with the enqueued notification we found.
-     */
-    else
-    {
-        int rc2 = getNotificationWriteOut(paParms, prop);
-        if (RT_FAILURE(rc2))
-            rc = rc2;
-    }
+        else
+        {
+            int rc2 = getNotificationWriteOut(cParms, paParms, prop);
+            if (RT_SUCCESS(rc))
+                rc = rc2;
+        }
+    }
+
+    LogFlowThisFunc(("returning rc=%Rrc\n", rc));
     return rc;
 }
@@ -1097 +1115 @@
  * @thread  HGCM service
  */
-void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
-{
-    AssertPtrReturnVoid(pszProperty);
-    LogFlowThisFunc (("pszProperty=%s, u64Timestamp=%llu\n", pszProperty, u64Timestamp));
+int Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
+{
+    AssertPtrReturn(pszProperty, VERR_INVALID_POINTER);
+    LogFlowThisFunc(("pszProperty=%s, u64Timestamp=%llu\n", pszProperty, u64Timestamp));
     /* Ensure that our timestamp is different to the last one. */
     if (   !mGuestNotifications.empty()
@@ -1122 +1140 @@
     }
 
-    /* Release waiters if applicable and add the event to the queue for
-     * guest notifications */
+    /* Release guest waiters if applicable and add the event
+     * to the queue for guest notifications */
     int rc = VINF_SUCCESS;
     try
@@ -1136 +1154 @@
             {
                 GuestCall curCall = *it;
-                int rc2 = getNotificationWriteOut(curCall.mParms, prop);
+                int rc2 = getNotificationWriteOut(curCall.mParmsCnt, curCall.mParms, prop);
                 if (RT_SUCCESS(rc2))
                     rc2 = curCall.mRc;
@@ -1145 +1163 @@
                 ++it;
         }
+
         mGuestNotifications.push_back(prop);
+
+        /** @todo r=andy This list does not have a purpose but for tracking
+          *              the timestamps ...
+        if (mGuestNotifications.size() > MAX_GUEST_NOTIFICATIONS)
+            mGuestNotifications.pop_front();
     }
     catch (std::bad_alloc)
@@ -1151 +1175 @@
         rc = VERR_NO_MEMORY;
     }
-    if (mGuestNotifications.size() > MAX_GUEST_NOTIFICATIONS)
-        mGuestNotifications.pop_front();
-
-    /*
-     * Host notifications - first case: if the property exists then send its
-     * current value
-     */
-    if (pProp && mpfnHostCallback != NULL)
-    {
-        char szFlags[MAX_FLAGS_LEN];
-        /* Send out a host notification */
-        const char *pszValue = prop.mValue.c_str();
-        if (RT_SUCCESS(rc))
+
+    if (   RT_SUCCESS(rc)
+        && mpfnHostCallback)
+    {
+        /*
+         * Host notifications - first case: if the property exists then send its
+         * current value
+         */
+        if (pProp)
+        {
+            char szFlags[MAX_FLAGS_LEN];
+            /* Send out a host notification */
+            const char *pszValue = prop.mValue.c_str();
             rc = writeFlags(prop.mFlags, szFlags);
-        if (RT_SUCCESS(rc))
-            rc = notifyHost(pszProperty, pszValue, u64Timestamp, szFlags);
-    }
-
-    /*
-     * Host notifications - second case: if the property does not exist then
-     * send the host an empty value
-     */
-    if (!pProp && mpfnHostCallback != NULL)
-    {
-        /* Send out a host notification */
-        if (RT_SUCCESS(rc))
+            if (RT_SUCCESS(rc))
+                rc = notifyHost(pszProperty, pszValue, u64Timestamp, szFlags);
+        }
+        /*
+         * Host notifications - second case: if the property does not exist then
+         * send the host an empty value
+         */
+        else
+        {
+            /* Send out a host notification */
             rc = notifyHost(pszProperty, "", u64Timestamp, "");
-    }
-    LogFlowThisFunc(("returning\n"));
+        }
+    }
+
+    LogFlowThisFunc(("returning rc=%Rrc\n", rc));
+    return rc;
 }
 
@@ -1201 +1226 @@
     HostCallbackData.u64Timestamp = u64Timestamp;
     HostCallbackData.pcszFlags    = pszFlags;
-    int rc = mpfnHostCallback (mpvHostData, 0 /*u32Function*/,
-                           (void *)(&HostCallbackData),
-                           sizeof(HostCallbackData));
-    LogFlowFunc (("returning %Rrc\n", rc));
+    int rc = mpfnHostCallback(mpvHostData, 0 /*u32Function*/,
+                              (void *)(&HostCallbackData),
+                              sizeof(HostCallbackData));
+    LogFlowFunc(("returning rc=%Rrc\n", rc));
     return rc;
 }