Imported from /home/lorry/working-area/delta_VirtualBox/VirtualBox-4.3.10.tar.bz2.HEADVirtualBox-4.3.10master

2 files changed, 125 insertions, 100 deletions

diff --git a/src/VBox/HostServices/GuestProperties/service.cpp b/src/VBox/HostServices/GuestProperties/service.cpp
index addcbec5..bd3a1626 100644
--- a/src/VBox/HostServices/GuestProperties/service.cpp
+++ b/src/VBox/HostServices/GuestProperties/service.cpp
@@ -4,7 +4,7 @@
  */
 
 /*
- * Copyright (C) 2008 Oracle Corporation
+ * Copyright (C) 2008-2013 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
  * available from http://www.virtualbox.org. This file is free software;
@@ -138,18 +138,20 @@ struct GuestCall
     VBOXHGCMCALLHANDLE mHandle;
     /** The function that was requested */
     uint32_t mFunction;
+    /** Number of call parameters. */
+    uint32_t mParmsCnt;
     /** The call parameters */
     VBOXHGCMSVCPARM *mParms;
     /** The default return value, used for passing warnings */
     int mRc;
 
     /** 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 */
 typedef std::list <GuestCall> CallList;
@@ -170,7 +172,8 @@ private:
     RTSTRSPACE mhProperties;
     /** 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 */
     CallList mGuestWaiters;
@@ -375,8 +378,8 @@ private:
                         VBOXHGCMSVCPARM paParms[]);
     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);
 
@@ -701,35 +704,41 @@ int Service::setProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGues
         }
         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++;
                 else
                 {
                     AssertFailed();
                     delete pProp;
-                    rc = VERR_INTERNAL_ERROR_3;
+
+                    rc = VERR_ALREADY_EXISTS;
                 }
             }
-            else
+            catch (std::bad_alloc)
+            {
                 rc = VERR_NO_MEMORY;
+            }
         }
         else
             rc = VERR_TOO_MUCH_DATA;
 
         /*
-         * 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));
+        int rc2 = doNotifications(pcszName, u64TimeNano);
+        if (RT_SUCCESS(rc))
+            rc = rc2;
     }
 
-    LogFlowThisFunc(("rc = %Rrc (%s = %s)\n", rc, pcszName, pcszValue));
+    LogFlowThisFunc(("%s=%s, rc=%Rrc\n", pcszName, pcszValue, rc));
     return rc;
 }
 
@@ -763,7 +772,7 @@ int Service::delProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGues
         rc = VERR_INVALID_PARAMETER;
     if (RT_FAILURE(rc))
     {
-        LogFlowThisFunc(("rc = %Rrc\n", rc));
+        LogFlowThisFunc(("rc=%Rrc\n", rc));
         return rc;
     }
 
@@ -787,10 +796,12 @@ int Service::delProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGues
         delete pProp;
         // if (isGuest)  /* Notify the host even for properties that the host
         //                * changed.  Less efficient, but ensures consistency. */
-            doNotifications(pcszName, u64Timestamp);
+        int rc2 = doNotifications(pcszName, u64Timestamp);
+        if (RT_SUCCESS(rc))
+            rc = rc2;
     }
 
-    LogFlowThisFunc(("rc = %Rrc (%s)\n", rc, pcszName));
+    LogFlowThisFunc(("%s: rc=%Rrc\n", pcszName, rc));
     return rc;
 }
 
@@ -967,15 +978,17 @@ int Service::getOldNotificationInternal(const char *pszPatterns,
 
 
 /** Helper query used by getNotification */
-int Service::getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop)
+int Service::getNotificationWriteOut(uint32_t cParms, VBOXHGCMSVCPARM paParms[], Property prop)
 {
-    int rc = VINF_SUCCESS;
+    AssertReturn(cParms == 4, VERR_INVALID_PARAMETER); /* Basic sanity checking. */
+
     /* Format the data to write to the buffer. */
     std::string buffer;
     uint64_t u64Timestamp;
     char *pchBuf;
     uint32_t cbBuf;
-    rc = paParms[2].getBuffer((void **)&pchBuf, &cbBuf);
+
+    int rc = paParms[2].getBuffer((void **)&pchBuf, &cbBuf);
     if (RT_SUCCESS(rc))
     {
         char szFlags[MAX_FLAGS_LEN];
@@ -1014,8 +1027,8 @@ int Service::getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop)
  * @thread  HGCM
  * @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;
     char *pszPatterns = NULL;           /* shut up gcc */
@@ -1034,9 +1047,9 @@ int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle
         || RT_FAILURE(paParms[2].getBuffer((void **)&pchBuf, &cbBuf))  /* return buffer */
        )
         rc = VERR_INVALID_PARAMETER;
+
     if (RT_SUCCESS(rc))
-        LogFlow(("    pszPatterns=%s, u64Timestamp=%llu\n", pszPatterns,
-                 u64Timestamp));
+        LogFlow(("pszPatterns=%s, u64Timestamp=%llu\n", pszPatterns, u64Timestamp));
 
     /*
      * If no timestamp was supplied or no notification was found in the queue
@@ -1045,45 +1058,50 @@ int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle
     Property prop;
     if (RT_SUCCESS(rc) && u64Timestamp != 0)
         rc = getOldNotification(pszPatterns, u64Timestamp, &prop);
-    if (RT_SUCCESS(rc) && prop.isNull())
+    if (RT_SUCCESS(rc))
     {
-        /*
-         * 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())
+        if (prop.isNull())
         {
-            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)
+            /*
+             * 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())
             {
-                /* Complete the old request. */
-                mpHelpers->pfnCallComplete(it->mHandle, VERR_INTERRUPTED);
-                it = mGuestWaiters.erase(it);
+                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;
             }
-            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;
+            mGuestWaiters.push_back(GuestCall(u32ClientId, callHandle, GET_NOTIFICATION,
+                                              cParms, paParms, rc));
+            rc = VINF_HGCM_ASYNC_EXECUTE;
+        }
+        /*
+         * Otherwise reply at once with the enqueued notification we found.
+         */
+        else
+        {
+            int rc2 = getNotificationWriteOut(cParms, paParms, prop);
+            if (RT_FAILURE(rc2))
+                rc = rc2;
+        }
     }
+
+    LogFlowThisFunc(("returning rc=%Rrc\n", rc));
     return rc;
 }
 
@@ -1096,10 +1114,10 @@ int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle
  *
  * @thread  HGCM service
  */
-void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
+int Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
 {
-    AssertPtrReturnVoid(pszProperty);
-    LogFlowThisFunc (("pszProperty=%s, u64Timestamp=%llu\n", pszProperty, 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()
         && u64Timestamp == mGuestNotifications.back().mTimestamp)
@@ -1121,8 +1139,8 @@ void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
         prop.mFlags = pProp->mFlags;
     }
 
-    /* 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
     {
@@ -1135,7 +1153,7 @@ void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
             if (prop.Matches(pszPatterns))
             {
                 GuestCall curCall = *it;
-                int rc2 = getNotificationWriteOut(curCall.mParms, prop);
+                int rc2 = getNotificationWriteOut(curCall.mParmsCnt, curCall.mParms, prop);
                 if (RT_SUCCESS(rc2))
                     rc2 = curCall.mRc;
                 mpHelpers->pfnCallComplete(curCall.mHandle, rc2);
@@ -1144,41 +1162,48 @@ void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp)
             else
                 ++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)
     {
         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)
+    if (   RT_SUCCESS(rc)
+        && mpfnHostCallback)
     {
-        char szFlags[MAX_FLAGS_LEN];
-        /* Send out a host notification */
-        const char *pszValue = prop.mValue.c_str();
-        if (RT_SUCCESS(rc))
+        /*
+         * 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;
 }
 
 /**
@@ -1200,10 +1225,10 @@ int Service::notifyHost(const char *pszName, const char *pszValue,
     HostCallbackData.pcszValue    = pszValue;
     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;
 }
 
diff --git a/src/VBox/HostServices/GuestProperties/testcase/tstGuestPropSvc.cpp b/src/VBox/HostServices/GuestProperties/testcase/tstGuestPropSvc.cpp
index 782fd87a..a1d3c387 100644
--- a/src/VBox/HostServices/GuestProperties/testcase/tstGuestPropSvc.cpp
+++ b/src/VBox/HostServices/GuestProperties/testcase/tstGuestPropSvc.cpp
@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright (C) 2008 Oracle Corporation
+ * Copyright (C) 2008-2012 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
  * available from http://www.virtualbox.org. This file is free software;
@@ -431,7 +431,7 @@ int doSetProperty(VBOXHGCMSVCFNTABLE *pTable, const char *pcszName,
  */
 static void testSetProp(VBOXHGCMSVCFNTABLE *pTable)
 {
-    RTTestISub("SET_PROP, SET_PROP_VALUE, SET_PROP_HOST, SET_PROP_VALUE_HOST");
+    RTTestISub("SET_PROP, _VALUE, _HOST, _VALUE_HOST");
 
     /** Array of properties for testing SET_PROP_HOST and _GUEST. */
     static const struct
@@ -745,7 +745,7 @@ struct asyncNotification_
  */
 static void setupAsyncNotification(VBOXHGCMSVCFNTABLE *pTable)
 {
-    RTTestISub("Asynchronous GET_NOTIFICATION call with no notifications are available");
+    RTTestISub("Async GET_NOTIFICATION without notifications");
     static char s_szPattern[] = "";
 
     g_AsyncNotification.aParms[0].setPointer((void *)s_szPattern, sizeof(s_szPattern));
@@ -796,7 +796,7 @@ static void test2(void)
     /* Set up the asynchronous notification test */
     setupAsyncNotification(&svcTable);
     testSetProp(&svcTable);
-    RTTestISub("Checking the data returned by the asynchronous notification call");
+    RTTestISub("Async notification call data");
     testAsyncNotification(&svcTable); /* Our previous notification call should have completed by now. */
 
     testDelProp(&svcTable);
@@ -838,7 +838,7 @@ static int doSetGlobalFlags(VBOXHGCMSVCFNTABLE *pTable, ePropFlags eFlags)
  */
 static void testSetPropROGuest(VBOXHGCMSVCFNTABLE *pTable)
 {
-    RTTestISub("SET_PROP, SET_PROP_VALUE, SET_PROP_HOST and SET_PROP_VALUE_HOST calls with READONLYGUEST set globally");
+    RTTestISub("global READONLYGUEST and SET_PROP*");
 
     /** Array of properties for testing SET_PROP_HOST and _GUEST with the
      * READONLYGUEST global flag set. */
@@ -908,7 +908,7 @@ static void testSetPropROGuest(VBOXHGCMSVCFNTABLE *pTable)
  */
 static void testDelPropROGuest(VBOXHGCMSVCFNTABLE *pTable)
 {
-    RTTestISub("DEL_PROP and DEL_PROP_HOST calls with RDONLYGUEST set globally");
+    RTTestISub("global READONLYGUEST and DEL_PROP*");
 
     /** Array of properties for testing DEL_PROP_HOST and _GUEST with
      * READONLYGUEST set globally. */