Changeset 13759 in vbox

Timestamp:

Nov 3, 2008 4:13:04 PM (16 years ago)

Author:

vboxsync

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

38734

Message:

Guest Properties (HostServices and Main): major clean up of the guest property service

Location:

trunk

Files:

• include/VBox/HostServices/GuestPropertySvc.h (modified) (4 diffs)
• src/VBox/HostServices/GuestProperties/service.cpp (modified) (26 diffs)
• src/VBox/HostServices/GuestProperties/testcase/Makefile.kmk (modified) (1 diff)
• src/VBox/HostServices/GuestProperties/testcase/tstGuestPropSvc.cpp (modified) (6 diffs)
• src/VBox/Main/ConsoleImpl.cpp (modified) (5 diffs)
• src/VBox/Main/ConsoleImpl2.cpp (modified) (4 diffs)
• src/VBox/Main/MachineImpl.cpp (modified) (2 diffs)
• src/VBox/Main/include/ConsoleImpl.h (modified) (1 diff)

trunk/include/VBox/HostServices/GuestPropertySvc.h

@@ -148 +148 @@
                 break;
         if (RT_ELEMENTS(flagList) == i)
-             rc = VERR_INVALID_PARAMETER;
+             rc = VERR_PARSE_ERROR;
         else
         {
@@ -157 +157 @@
             if (',' == *pcszNext)
                 ++pcszNext;
+            else if (*pcszNext != '\0')
+                rc = VERR_PARSE_ERROR;
             while (' ' == *pcszNext)
                 ++pcszNext;
@@ -189 +191 @@
         for (; i < RT_ELEMENTS(flagList); ++i)
         {
-            if (fFlags & flagList[i])
+            if (flagList[i] == (fFlags & flagList[i]))
             {
                 strcpy(pszNext, flagName(flagList[i]));
@@ -213 +215 @@
 enum eHostFn
 {
-    /** Pass the address of the cfgm node used by the service as a database. */
-    SET_CFGM_NODE = 1,
+    /**
+     * Set properties in a block.  The parameters are pointers to
+     * NULL-terminated arrays containing the paramters.  These are, in order,
+     * name, value, timestamp, flags.  Strings are stored as pointers to
+     * mutable utf8 data.  All parameters must be supplied.
+     */
+    SET_PROPS_HOST = 1,
     /**
      * Get the value attached to a guest property

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

@@ -47 +47 @@
 
 #include <memory>  /* for auto_ptr */
+#include <string>
+#include <list>
 
 #include <iprt/err.h>
@@ -59 +61 @@
 #include <VBox/log.h>
 
-#include <VBox/cfgm.h>
-
 /*******************************************************************************
 *   Internal functions                                                         *
@@ -77 +77 @@
 }
 
+/** Extract a constant pointer value from an HGCM parameter structure */
+static int VBoxHGCMParmPtrConstGet (VBOXHGCMSVCPARM *pParm, const void **ppv, uint32_t *pcb)
+{
+    if (pParm->type == VBOX_HGCM_SVC_PARM_PTR)
+    {
+        *ppv = pParm->u.pointer.addr;
+        *pcb = pParm->u.pointer.size;
+        return VINF_SUCCESS;
+    }
+
+    return VERR_INVALID_PARAMETER;
+}
+
 /** Set a uint32_t value to an HGCM parameter structure */
 static void VBoxHGCMParmUInt32Set (VBOXHGCMSVCPARM *pParm, uint32_t u32)
@@ -84 +97 @@
 }
 
-
 /** Set a uint64_t value to an HGCM parameter structure */
 static void VBoxHGCMParmUInt64Set (VBOXHGCMSVCPARM *pParm, uint64_t u64)
@@ -90 +102 @@
     pParm->type = VBOX_HGCM_SVC_PARM_64BIT;
     pParm->u.uint64 = u64;
+}
+
+/** Set a pointer value to an HGCM parameter structure */
+static void VBoxHGCMParmPtrSet (VBOXHGCMSVCPARM *pParm, void *pv, uint32_t cb)
+{
+    pParm->type = VBOX_HGCM_SVC_PARM_PTR;
+    pParm->u.pointer.addr = pv;
+    pParm->u.pointer.size = cb;
 }
 
@@ -104 +124 @@
     /** HGCM helper functions. */
     PVBOXHGCMSVCHELPERS mpHelpers;
-    /** Pointer to our configuration values node. */
-    PCFGMNODE mpValueNode;
-    /** Pointer to our configuration timestamps node. */
-    PCFGMNODE mpTimestampNode;
-    /** Pointer to our configuration flags node. */
-    PCFGMNODE mpFlagsNode;
+    /** 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;
+        /** 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;
     /** @todo we should have classes for thread and request handler thread */
     /** Queue of outstanding property change notifications */
@@ -125 +165 @@
 public:
     explicit Service(PVBOXHGCMSVCHELPERS pHelpers)
-        : mpHelpers(pHelpers), mpValueNode(NULL), mpTimestampNode(NULL),
-          mpFlagsNode(NULL), mfExitThread(false), mpfnHostCallback(NULL),
+        : mpHelpers(pHelpers), mfExitThread(false), mpfnHostCallback(NULL),
           mpvHostData(NULL)
     {
         int rc = RTReqCreateQueue(&mReqQueue);
-        rc = RTThreadCreate(&mReqThread, reqThreadFn, this, 0, RTTHREADTYPE_MSG_PUMP,
-                                RTTHREADFLAGS_WAITABLE, "GuestPropReq");
+#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
+        if (RT_SUCCESS(rc))
+            rc = RTThreadCreate(&mReqThread, reqThreadFn, this, 0,
+                                RTTHREADTYPE_MSG_PUMP, RTTHREADFLAGS_WAITABLE,
+                                "GuestPropReq");
+#endif
         if (RT_FAILURE(rc))
             throw rc;
@@ -210 +253 @@
     static DECLCALLBACK(int) reqThreadFn(RTTHREAD ThreadSelf, void *pvUser);
     int validateName(const char *pszName, uint32_t cbName);
-    int validateValue(char *pszValue, uint32_t cbValue);
-    int getPropValue(uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
+    int validateValue(const char *pszValue, uint32_t cbValue);
+    int setPropertyBlock(uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
     int getProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
     int setProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest);
@@ -286 +329 @@
  * @thread  HGCM
  */
-int Service::validateValue(char *pszValue, uint32_t cbValue)
+int Service::validateValue(const char *pszValue, uint32_t cbValue)
 {
     LogFlowFunc(("cbValue=%d\n", cbValue));
@@ -305 +348 @@
 }
 
+/**
+ * Set a block of properties in the property registry, checking the validity
+ * of the arguments passed.
+ *
+ * @returns iprt status value
+ * @param   cParms  the number of HGCM parameters supplied
+ * @param   paParms the array of HGCM parameters
+ * @thread  HGCM
+ */
+int Service::setPropertyBlock(uint32_t cParms, VBOXHGCMSVCPARM paParms[])
+{
+    char **ppNames, **ppValues, **ppFlags;
+    uint64_t *pTimestamps;
+    uint32_t cbDummy;
+    int rc = VINF_SUCCESS;
+
+    /*
+     * Get and validate the parameters
+     */
+    if (   (cParms != 4)
+        || RT_FAILURE(VBoxHGCMParmPtrGet (&paParms[0], (void **) &ppNames, &cbDummy))
+        || RT_FAILURE(VBoxHGCMParmPtrGet (&paParms[1], (void **) &ppValues, &cbDummy))
+        || RT_FAILURE(VBoxHGCMParmPtrGet (&paParms[2], (void **) &pTimestamps, &cbDummy))
+        || RT_FAILURE(VBoxHGCMParmPtrGet (&paParms[3], (void **) &ppFlags, &cbDummy))
+        )
+        rc = VERR_INVALID_PARAMETER;
+
+    /*
+     * Add the properties to the end of the list.  If we succeed then we
+     * will remove duplicates afterwards.
+     */
+    /* Remember the last property before we started adding, for rollback or
+     * cleanup. */
+    PropertyList::iterator itEnd = mProperties.end();
+    if (!mProperties.empty())
+        --itEnd;
+    try
+    {
+        for (unsigned i = 0; RT_SUCCESS(rc) && ppNames[i] != NULL; ++i)
+        {
+            uint32_t fFlags;
+            if (   !VALID_PTR(ppNames[i])
+                || !VALID_PTR(ppValues[i])
+                || !VALID_PTR(ppFlags[i])
+              )
+                rc = VERR_INVALID_POINTER;
+            if (RT_SUCCESS(rc))
+                rc = validateFlags(ppFlags[i], &fFlags);
+            if (RT_SUCCESS(rc))
+                mProperties.push_back(Property(ppNames[i], ppValues[i],
+                                               pTimestamps[i], fFlags));
+        }
+    }
+    catch (std::bad_alloc)
+    {
+        rc = VERR_NO_MEMORY;
+    }
+
+    /*
+     * If all went well then remove the duplicate elements.
+     */
+    if (RT_SUCCESS(rc) && itEnd != mProperties.end())
+    {
+        ++itEnd;
+        for (unsigned i = 0; ppNames[i] != NULL; ++i)
+        {
+            bool found = false;
+            for (PropertyList::iterator it = mProperties.begin();
+                !found && it != itEnd; ++it)
+                if (it->mName.compare(ppNames[i]) == 0)
+                {
+                    found = true;
+                    mProperties.erase(it);
+                }
+        }
+    }
+
+    /*
+     * If something went wrong then rollback.  This is possible because we
+     * haven't deleted anything yet.
+     */
+    if (RT_FAILURE(rc))
+    {
+        if (itEnd != mProperties.end())
+            ++itEnd;
+        mProperties.erase(itEnd, mProperties.end());
+    }
+    return rc;
+}
 
 /**
@@ -318 +450 @@
  * @thread  HGCM
  */
-int Service::getPropValue(uint32_t cParms, VBOXHGCMSVCPARM paParms[])
+int Service::getProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[])
 {
     int rc = VINF_SUCCESS;
-    char *pszName, *pszValue;
-    uint32_t cbName, cbValue;
-    size_t cbValueActual;
-
-    LogFlowThisFunc(("\n"));
-    AssertReturn(VALID_PTR(mpValueNode), VERR_WRONG_ORDER);  /* a.k.a. VERR_NOT_INITIALIZED */
-    if (   (cParms != 3)  /* Hardcoded value as the next lines depend on it. */
-        || (paParms[0].type != VBOX_HGCM_SVC_PARM_PTR)   /* name */
-        || (paParms[1].type != VBOX_HGCM_SVC_PARM_PTR)   /* value */
-       )
-        rc = VERR_INVALID_PARAMETER;
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[0], (void **) &pszName, &cbName);
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[1], (void **) &pszValue, &cbValue);
-    if (RT_SUCCESS(rc))
-        rc = validateName(pszName, cbName);
-    if (RT_SUCCESS(rc))
-        rc = CFGMR3QuerySize(mpValueNode, pszName, &cbValueActual);
-    if (RT_SUCCESS(rc))
-        VBoxHGCMParmUInt32Set(&paParms[2], cbValueActual);
-    if (RT_SUCCESS(rc) && (cbValueActual > cbValue))
-        rc = VERR_BUFFER_OVERFLOW;
-    if (RT_SUCCESS(rc))
-        rc = CFGMR3QueryString(mpValueNode, pszName, pszValue, cbValue);
-    if (RT_SUCCESS(rc))
-        Log2(("Queried string %s, rc=%Rrc, value=%.*s\n", pszName, rc, cbValue, pszValue));
-    else if (VERR_CFGM_VALUE_NOT_FOUND == rc)
-        rc = VERR_NOT_FOUND;
-    LogFlowThisFunc(("rc = %Rrc\n", rc));
-    return rc;
-}
-
-
-/**
- * Retrieve a value from the property registry by name, checking the validity
- * of the arguments passed.  If the guest has not allocated enough buffer
- * space for the value then we return VERR_OVERFLOW and set the size of the
- * buffer needed in the "size" HGCM parameter.  If the name was not found at
- * all, we return VERR_NOT_FOUND.
- *
- * @returns iprt status value
- * @param   cParms  the number of HGCM parameters supplied
- * @param   paParms the array of HGCM parameters
- * @thread  HGCM
- */
-int Service::getProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[])
-{
-    int rc = VINF_SUCCESS;
-    char *pszName, *pchBuf;
+    const char *pcszName;
+    char *pchBuf;
     uint32_t cchName, cchBuf;
-    size_t cchValue, cchFlags, cchBufActual;
+    size_t cchFlags, cchBufActual;
     char szFlags[MAX_FLAGS_LEN];
     uint32_t fFlags;
@@ -380 +464 @@
      */
     LogFlowThisFunc(("\n"));
-    AssertReturn(VALID_PTR(mpValueNode), VERR_WRONG_ORDER);  /* a.k.a. VERR_NOT_INITIALIZED */
     if (   (cParms != 4)  /* Hardcoded value as the next lines depend on it. */
         || (paParms[0].type != VBOX_HGCM_SVC_PARM_PTR)    /* name */
@@ -387 +470 @@
         rc = VERR_INVALID_PARAMETER;
     if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[0], (void **) &pszName, &cchName);
+        rc = VBoxHGCMParmPtrConstGet(&paParms[0], (const void **) &pcszName, &cchName);
     if (RT_SUCCESS(rc))
         rc = VBoxHGCMParmPtrGet(&paParms[1], (void **) &pchBuf, &cchBuf);
     if (RT_SUCCESS(rc))
-        rc = validateName(pszName, cchName);
+        rc = validateName(pcszName, cchName);
 
     /*
@@ -398 +481 @@
 
     /* Get the value size */
-    if (RT_SUCCESS(rc))
-        rc = CFGMR3QuerySize(mpValueNode, pszName, &cchValue);
-    /* Get the flags and their size */
-    if (RT_SUCCESS(rc))
-        CFGMR3QueryU32(mpFlagsNode, pszName, (uint32_t *)&fFlags);
-    if (RT_SUCCESS(rc))
-        rc = writeFlags(fFlags, szFlags);
+    PropertyList::const_iterator it;
+    bool found = false;
+    if (RT_SUCCESS(rc))
+        for (it = mProperties.begin(); it != mProperties.end(); ++it)
+            if (it->mName.compare(pcszName) == 0)
+            {
+                found = true;
+                break;
+            }
+    if (RT_SUCCESS(rc) && !found)
+        rc = VERR_NOT_FOUND;
+    if (RT_SUCCESS(rc))
+        rc = writeFlags(it->mFlags, szFlags);
     if (RT_SUCCESS(rc))
         cchFlags = strlen(szFlags);
@@ -410 +499 @@
     if (RT_SUCCESS(rc))
     {
-        cchBufActual = cchValue + cchFlags;
+        cchBufActual = it->mValue.size() + 1 + cchFlags;
         VBoxHGCMParmUInt32Set(&paParms[3], cchBufActual);
     }
     if (RT_SUCCESS(rc) && (cchBufActual > cchBuf))
         rc = VERR_BUFFER_OVERFLOW;
-    /* Write the value */
-    if (RT_SUCCESS(rc))
-        rc = CFGMR3QueryString(mpValueNode, pszName, pchBuf, cchBuf);
-    /* Write the flags */
-    if (RT_SUCCESS(rc))
-        strcpy(pchBuf + cchValue, szFlags);
-    /* Timestamp */
-    uint64_t u64Timestamp = 0;
-    if (RT_SUCCESS(rc))
-        CFGMR3QueryU64(mpTimestampNode, pszName, &u64Timestamp);
-    VBoxHGCMParmUInt64Set(&paParms[2], u64Timestamp);
+    /* Write the value, flags and timestamp */
+    if (RT_SUCCESS(rc))
+    {
+        it->mValue.copy(pchBuf, cchBuf, 0);
+        pchBuf[it->mValue.size()] = '\0'; /* Terminate the value */
+        strcpy(pchBuf + it->mValue.size() + 1, szFlags);
+        VBoxHGCMParmUInt64Set(&paParms[2], it->mTimestamp);
+    }
 
     /*
@@ -431 +517 @@
      */
     if (RT_SUCCESS(rc))
-        Log2(("Queried string %S, value=%.*S, timestamp=%lld, flags=%.*S\n",
-              pszName, cchValue, pchBuf, u64Timestamp, cchFlags,
-              pchBuf + cchValue));
-    else if (VERR_CFGM_VALUE_NOT_FOUND == rc)
-        rc = VERR_NOT_FOUND;
+        Log2(("Queried string %s, value=%s, timestamp=%lld, flags=%s\n",
+              pcszName, it->mValue.c_str(), it->mTimestamp, szFlags));
     LogFlowThisFunc(("rc = %Rrc\n", rc));
     return rc;
 }
 
-
 /**
  * Set a value in the property registry by name, checking the validity
+ * of the arguments passed.
+ *
+ * @returns iprt status value
+ * @param   cParms  the number of HGCM parameters supplied
+ * @param   paParms the array of HGCM parameters
+ * @param   isGuest is this call coming from the guest (or the host)?
+ * @throws  std::bad_alloc  if an out of memory condition occurs
+ * @thread  HGCM
+ */
+int Service::setProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest)
+{
+    int rc = VINF_SUCCESS;
+    const char *pcszName, *pcszValue, *pcszFlags = NULL;
+    uint32_t cchName, cchValue, cchFlags = 0;
+    uint32_t fFlags = NILFLAG;
+
+    LogFlowThisFunc(("\n"));
+    /*
+     * First of all, make sure that we won't exceed the maximum number of properties.
+     */
+    if (mProperties.size() >= MAX_PROPS)
+        rc = VERR_TOO_MUCH_DATA;
+
+    /*
+     * General parameter correctness checking.
+     */
+    if (   RT_SUCCESS(rc)
+        && (   (cParms < 2) || (cParms > 3)  /* Hardcoded value as the next lines depend on it. */
+            || RT_FAILURE(VBoxHGCMParmPtrConstGet(&paParms[0],
+                   (const void **) &pcszName, &cchName)) /* name */
+            || RT_FAILURE(VBoxHGCMParmPtrConstGet(&paParms[1],
+                   (const void **) &pcszValue, &cchValue)) /* value */
+            || (   (3 == cParms)
+                && RT_FAILURE(VBoxHGCMParmPtrConstGet(&paParms[2],
+                       (const void **) &pcszFlags, &cchFlags)) /* flags */
+               )
+           )
+       )
+        rc = VERR_INVALID_PARAMETER;
+
+    /*
+     * Check the values passed in the parameters for correctness.
+     */
+    if (RT_SUCCESS(rc))
+        rc = validateName(pcszName, cchName);
+    if (RT_SUCCESS(rc))
+        rc = validateValue(pcszValue, cchValue);
+    if ((3 == cParms) && RT_SUCCESS(rc))
+        rc = RTStrValidateEncodingEx(pcszFlags, cchFlags,
+                                     RTSTR_VALIDATE_ENCODING_ZERO_TERMINATED);
+    if ((3 == cParms) && RT_SUCCESS(rc))
+        rc = validateFlags(pcszFlags, &fFlags);
+
+    /*
+     * If the property already exists, check its flags to see if we are allowed
+     * to change it.
+     */
+    PropertyList::iterator it;
+    bool found = false;
+    if (RT_SUCCESS(rc))
+        for (it = mProperties.begin(); it != mProperties.end(); ++it)
+            if (it->mName.compare(pcszName) == 0)
+            {
+                found = true;
+                break;
+            }
+    if (RT_SUCCESS(rc) && found)
+        if (   (isGuest && (it->mFlags & RDONLYGUEST))
+            || (!isGuest && (it->mFlags & RDONLYHOST))
+           )
+            rc = VERR_PERMISSION_DENIED;
+
+    /*
+     * Set the actual value
+     */
+    if (RT_SUCCESS(rc))
+    {
+        RTTIMESPEC time;
+        uint64_t u64TimeNano = RTTimeSpecGetNano(RTTimeNow(&time));
+        if (found)
+        {
+            it->mValue = pcszValue;
+            it->mTimestamp = u64TimeNano;
+            it->mFlags = fFlags;
+        }
+        else  /* This can throw.  No problem as we have nothing to roll back. */
+            mProperties.push_back(Property(pcszName, pcszValue, u64TimeNano, fFlags));
+    }
+
+    /*
+     * Send a notification to the host and return.
+     */
+    if (RT_SUCCESS(rc))
+    {
+        // if (isGuest)  /* Notify the host even for properties that the host
+        //                * changed.  Less efficient, but ensures consistency. */
+            notifyHost(pcszName);
+        Log2(("Set string %s, rc=%Rrc, value=%s\n", pcszName, rc, pcszValue));
+    }
+    LogFlowThisFunc(("rc = %Rrc\n", rc));
+    return rc;
+}
+
+
+/**
+ * Remove a value in the property registry by name, checking the validity
+ * of the arguments passed.
+ *
+ * @returns iprt status value
+ * @param   cParms  the number of HGCM parameters supplied
+ * @param   paParms the array of HGCM parameters
+ * @param   isGuest is this call coming from the guest (or the host)?
+ * @thread  HGCM
+ */
+int Service::delProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest)
+{
+    int rc = VINF_SUCCESS;
+    const char *pcszName;
+    uint32_t cbName;
+
+    LogFlowThisFunc(("\n"));
+
+    /*
+     * Check the user-supplied parameters.
+     */
+    if (   (cParms != 1)  /* Hardcoded value as the next lines depend on it. */
+        || RT_FAILURE(VBoxHGCMParmPtrConstGet(&paParms[0],
+               (const void **) &pcszName, &cbName))  /* name */
+       )
+        rc = VERR_INVALID_PARAMETER;
+    if (RT_SUCCESS(rc))
+        rc = validateName(pcszName, cbName);
+
+    /*
+     * If the property exists, check its flags to see if we are allowed
+     * to change it.
+     */
+    PropertyList::iterator it;
+    bool found = false;
+    if (RT_SUCCESS(rc))
+        for (it = mProperties.begin(); it != mProperties.end(); ++it)
+            if (it->mName.compare(pcszName) == 0)
+            {
+                found = true;
+                break;
+            }
+    if (RT_SUCCESS(rc) && found)
+        if (   (isGuest && (it->mFlags & RDONLYGUEST))
+            || (!isGuest && (it->mFlags & RDONLYHOST))
+           )
+            rc = VERR_PERMISSION_DENIED;
+
+    /*
+     * And delete the property if all is well.
+     */
+    if (RT_SUCCESS(rc) && found)
+    {
+        mProperties.erase(it);
+        // if (isGuest)  /* Notify the host even for properties that the host
+        //                * changed.  Less efficient, but ensures consistency. */
+            notifyHost(pcszName);
+    }
+    LogFlowThisFunc(("rc = %Rrc\n", rc));
+    return rc;
+}
+
+/**
+ * Enumerate guest properties by mask, checking the validity
  * of the arguments passed.
  *
@@ -450 +700 @@
  * @thread  HGCM
  */
-int Service::setProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest)
+int Service::enumProps(uint32_t cParms, VBOXHGCMSVCPARM paParms[])
 {
     int rc = VINF_SUCCESS;
-    char *pszName, *pszValue;
-    uint32_t cchName, cchValue;
-    uint32_t fFlags = NILFLAG;
-
+
+    /*
+     * Get the HGCM function arguments.
+     */
+    char *pcchPatterns = NULL, *pchBuf = NULL;
+    uint32_t cchPatterns = 0, cchBuf = 0;
     LogFlowThisFunc(("\n"));
-    AssertReturn(VALID_PTR(mpValueNode), VERR_WRONG_ORDER);  /* a.k.a. VERR_NOT_INITIALIZED */
-    /*
-     * First of all, make sure that we won't exceed the maximum number of properties.
-     */
-    {
-        unsigned cChildren = 0;
-        for (PCFGMNODE pChild = CFGMR3GetFirstChild(mpValueNode); pChild != 0; pChild = CFGMR3GetNextChild(pChild))
-            ++cChildren;
-        if (cChildren >= MAX_PROPS)
-            rc = VERR_TOO_MUCH_DATA;
-    }
-    /*
-     * General parameter correctness checking.
-     */
-    if (   RT_SUCCESS(rc)
-        && (   (cParms < 2) || (cParms > 4)  /* Hardcoded value as the next lines depend on it. */
-            || (paParms[0].type != VBOX_HGCM_SVC_PARM_PTR)   /* name */
-            || (paParms[1].type != VBOX_HGCM_SVC_PARM_PTR)   /* value */
-            || ((3 == cParms) && (paParms[2].type != VBOX_HGCM_SVC_PARM_PTR))  /* flags */
-           )
+    if (   (cParms != 3)  /* Hardcoded value as the next lines depend on it. */
+        || RT_FAILURE(VBoxHGCMParmPtrConstGet(&paParms[0],
+               (const void **) &pcchPatterns, &cchPatterns))  /* patterns */
+        || RT_FAILURE(VBoxHGCMParmPtrGet(&paParms[1], 
+               (void **) &pchBuf, &cchBuf))  /* return buffer */
        )
         rc = VERR_INVALID_PARAMETER;
-    /*
-     * Check the values passed in the parameters for correctness.
-     */
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[0], (void **) &pszName, &cchName);
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[1], (void **) &pszValue, &cchValue);
-    if (RT_SUCCESS(rc))
-        rc = validateName(pszName, cchName);
-    if (RT_SUCCESS(rc))
-        rc = validateValue(pszValue, cchValue);
-
-    /*
-     * If the property already exists, check its flags to see if we are allowed
-     * to change it.
-     */
-    if (RT_SUCCESS(rc))
-    {
-        CFGMR3QueryU32(mpFlagsNode, pszName, &fFlags);  /* Failure is no problem here. */
-        if (   (isGuest && (fFlags & RDONLYGUEST))
-            || (!isGuest && (fFlags & RDONLYHOST))
-           )
-            rc = VERR_PERMISSION_DENIED;
-    }
-
-    /*
-     * Check whether the user supplied flags (if any) are valid.
-     */
-    if (RT_SUCCESS(rc) && (3 == cParms))
-    {
-        char *pszFlags;
-        uint32_t cchFlags;
-        rc = VBoxHGCMParmPtrGet(&paParms[2], (void **) &pszFlags, &cchFlags);
-        if (RT_SUCCESS(rc))
-            rc = validateFlags(pszFlags, &fFlags);
-    }
-    /*
-     * Set the actual value
-     */
-    if (RT_SUCCESS(rc))
-    {
-        RTTIMESPEC time;
-        CFGMR3RemoveValue(mpValueNode, pszName);
-        CFGMR3RemoveValue(mpTimestampNode, pszName);
-        CFGMR3RemoveValue(mpFlagsNode, pszName);
-        rc = CFGMR3InsertString(mpValueNode, pszName, pszValue);
-        if (RT_SUCCESS(rc))
-            rc = CFGMR3InsertInteger(mpTimestampNode, pszName,
-                                     RTTimeSpecGetNano(RTTimeNow(&time)));
-        if (RT_SUCCESS(rc))
-            rc = CFGMR3InsertInteger(mpFlagsNode, pszName, fFlags);
-        /* If anything goes wrong, make sure that we leave a clean state
-         * behind. */
-        if (RT_FAILURE(rc))
-        {
-            CFGMR3RemoveValue(mpValueNode, pszName);
-            CFGMR3RemoveValue(mpTimestampNode, pszName);
-            CFGMR3RemoveValue(mpFlagsNode, pszName);
-        }
-    }
-    /*
-     * Send a notification to the host and return.
-     */
-    if (RT_SUCCESS(rc))
-    {
-        if (isGuest)
-            notifyHost(pszName);
-        Log2(("Set string %s, rc=%Rrc, value=%s\n", pszName, rc, pszValue));
-    }
-    LogFlowThisFunc(("rc = %Rrc\n", rc));
-    return rc;
-}
-
-
-/**
- * Remove a value in the property registry by name, checking the validity
- * of the arguments passed.
- *
- * @returns iprt status value
- * @param   cParms  the number of HGCM parameters supplied
- * @param   paParms the array of HGCM parameters
- * @thread  HGCM
- */
-int Service::delProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest)
-{
-    int rc = VINF_SUCCESS;
-    char *pszName;
-    uint32_t cbName;
-
-    LogFlowThisFunc(("\n"));
-    AssertReturn(VALID_PTR(mpValueNode), VERR_WRONG_ORDER);  /* a.k.a. VERR_NOT_INITIALIZED */
-
-    /*
-     * Check the user-supplied parameters.
-     */
-    if (   (cParms != 1)  /* Hardcoded value as the next lines depend on it. */
-        || (paParms[0].type != VBOX_HGCM_SVC_PARM_PTR)   /* name */
-       )
-        rc = VERR_INVALID_PARAMETER;
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[0], (void **) &pszName, &cbName);
-    if (RT_SUCCESS(rc))
-        rc = validateName(pszName, cbName);
-
-    /*
-     * If the property already exists, check its flags to see if we are allowed
-     * to change it.
-     */
-    if (RT_SUCCESS(rc))
-    {
-        uint32_t fFlags = NILFLAG;
-        CFGMR3QueryU32(mpFlagsNode, pszName, &fFlags);  /* Failure is no problem here. */
-        if (   (isGuest && (fFlags & RDONLYGUEST))
-            || (!isGuest && (fFlags & RDONLYHOST))
-           )
-            rc = VERR_PERMISSION_DENIED;
-    }
-
-    /*
-     * And delete the property if all is well.
-     */
-    if (RT_SUCCESS(rc))
-    {
-        CFGMR3RemoveValue(mpValueNode, pszName);
-        if (isGuest)
-            notifyHost(pszName);
-    }
-    LogFlowThisFunc(("rc = %Rrc\n", rc));
-    return rc;
-}
-
-/**
- * Enumerate guest properties by mask, checking the validity
- * of the arguments passed.
- *
- * @returns iprt status value
- * @param   cParms  the number of HGCM parameters supplied
- * @param   paParms the array of HGCM parameters
- * @thread  HGCM
- */
-int Service::enumProps(uint32_t cParms, VBOXHGCMSVCPARM paParms[])
-{
-    /* We reallocate the temporary buffer in which we build up our array in
-     * increments of size BLOCK: */
-    enum
-    {
-        /* Calculate the increment, not yet rounded down */
-        BLOCKINCRFULL = (MAX_NAME_LEN + MAX_VALUE_LEN + MAX_FLAGS_LEN + 2048),
-        /* And this is the increment after rounding */
-        BLOCKINCR = BLOCKINCRFULL - BLOCKINCRFULL % 1024
-    };
-    int rc = VINF_SUCCESS;
-
-/*
- * Get the HGCM function arguments.
- */
-    char *paszPatterns = NULL, *pchBuf = NULL;
-    uint32_t cchPatterns = 0, cchBuf = 0;
-    LogFlowThisFunc(("\n"));
-    AssertReturn(VALID_PTR(mpValueNode), VERR_WRONG_ORDER);  /* a.k.a. VERR_NOT_INITIALIZED */
-    if (   (cParms != 3)  /* Hardcoded value as the next lines depend on it. */
-        || (paParms[0].type != VBOX_HGCM_SVC_PARM_PTR)   /* patterns */
-        || (paParms[1].type != VBOX_HGCM_SVC_PARM_PTR)   /* return buffer */
-       )
-        rc = VERR_INVALID_PARAMETER;
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[0], (void **) &paszPatterns, &cchPatterns);
     if (RT_SUCCESS(rc) && cchPatterns > MAX_PATTERN_LEN)
         rc = VERR_TOO_MUCH_DATA;
-    if (RT_SUCCESS(rc))
-        rc = VBoxHGCMParmPtrGet(&paParms[1], (void **) &pchBuf, &cchBuf);
-
-/*
- * First repack the patterns into the format expected by RTStrSimplePatternMatch()
- */
+
+    /*
+     * First repack the patterns into the format expected by RTStrSimplePatternMatch()
+     */
     bool matchAll = false;
     char pszPatterns[MAX_PATTERN_LEN];
-    if (   (NULL == paszPatterns)
+    if (   (NULL == pcchPatterns)
         || (cchPatterns < 2)  /* An empty pattern string means match all */
        )
@@ -664 +732 @@
     {
         for (unsigned i = 0; i < cchPatterns - 1; ++i)
-            if (paszPatterns[i] != '\0')
-                pszPatterns[i] = paszPatterns[i];
+            if (pcchPatterns[i] != '\0')
+                pszPatterns[i] = pcchPatterns[i];
             else
                 pszPatterns[i] = '|';
@@ -671 +739 @@
     }
 
-/*
- * Next enumerate all values in the current node into a temporary buffer.
- */
-    RTMemAutoPtr<char> TmpBuf;
-    uint32_t cchTmpBuf = 0, iTmpBuf = 0;
-    PCFGMLEAF pLeaf = CFGMR3GetFirstValue(mpValueNode);
-    while ((pLeaf != NULL) && RT_SUCCESS(rc))
-    {
-        /* Reallocate the buffer if it has got too tight */
-        if (iTmpBuf + BLOCKINCR > cchTmpBuf)
-        {
-            cchTmpBuf += BLOCKINCR * 2;
-            if (!TmpBuf.realloc(cchTmpBuf))
-                rc = VERR_NO_MEMORY;
-        }
-        /* Fetch the name into the buffer and if it matches one of the
-         * patterns, add its value and an empty timestamp and flags.  If it
-         * doesn't match, we simply overwrite it in the buffer. */
-        if (RT_SUCCESS(rc))
-            rc = CFGMR3GetValueName(pLeaf, &TmpBuf[iTmpBuf], cchTmpBuf - iTmpBuf);
-        /* Only increment the buffer offest if the name matches, otherwise we
-         * overwrite it next iteration. */
-        if (   RT_SUCCESS(rc)
-            && (   matchAll
-                || RTStrSimplePatternMultiMatch(pszPatterns, RTSTR_MAX,
-                                                &TmpBuf[iTmpBuf], RTSTR_MAX,
-                                                NULL)
-               )
+    /*
+     * Next enumerate into a temporary buffer.  This can throw, but this is
+     * not a problem as we have nothing to roll back.
+     */
+    std::string buffer;
+    for (PropertyList::const_iterator it = mProperties.begin();
+         RT_SUCCESS(rc) && (it != mProperties.end()); ++it)
+    {
+        if (   matchAll
+            || RTStrSimplePatternMultiMatch(pszPatterns, RTSTR_MAX,
+                                            it->mName.c_str(), RTSTR_MAX, NULL)
            )
         {
-            const char *pszName = &TmpBuf[iTmpBuf];
-            /* Get value */
-            iTmpBuf += strlen(&TmpBuf[iTmpBuf]) + 1;
-            rc = CFGMR3QueryString(mpValueNode, pszName, &TmpBuf[iTmpBuf],
-                                   cchTmpBuf - iTmpBuf);
+            char szFlags[MAX_FLAGS_LEN];
+            char szTimestamp[256];
+            size_t cchTimestamp;
+            buffer += it->mName;
+            buffer += '\0';
+            buffer += it->mValue;
+            buffer += '\0';
+            cchTimestamp = RTStrFormatNumber(szTimestamp, it->mTimestamp,
+                                             10, 0, 0, 0);
+            buffer.append(szTimestamp, cchTimestamp);
+            buffer += '\0';
+            rc = writeFlags(it->mFlags, szFlags);
             if (RT_SUCCESS(rc))
-            {
-                /* Get timestamp */
-                iTmpBuf += strlen(&TmpBuf[iTmpBuf]) + 1;
-                uint64_t u64Timestamp = 0;
-                CFGMR3QueryU64(mpTimestampNode, pszName, &u64Timestamp);
-                iTmpBuf += RTStrFormatNumber(&TmpBuf[iTmpBuf], u64Timestamp,
-                                             10, 0, 0, 0) + 1;
-                /* Get flags */
-                uint32_t fFlags = NILFLAG;
-                CFGMR3QueryU32(mpFlagsNode, pszName, &fFlags);
-                TmpBuf[iTmpBuf] = '\0';  /* Bad (== in)sanity, will be fixed. */
-                writeFlags(fFlags, &TmpBuf[iTmpBuf]);
-                iTmpBuf += strlen(&TmpBuf[iTmpBuf]) + 1;
-            }
+                buffer += szFlags;
+            buffer += '\0';
         }
-        if (RT_SUCCESS(rc))
-            pLeaf = CFGMR3GetNextValue(pLeaf);
-    }
-    if (RT_SUCCESS(rc))
-    {
-        /* The terminator.  We *do* have space left for this. */
-        TmpBuf[iTmpBuf] = '\0';
-        TmpBuf[iTmpBuf + 1] = '\0';
-        TmpBuf[iTmpBuf + 2] = '\0';
-        TmpBuf[iTmpBuf + 3] = '\0';
-        iTmpBuf += 4;
-        VBoxHGCMParmUInt32Set(&paParms[2], iTmpBuf);
+    }
+    buffer.append(4, '\0');  /* The final terminators */
+
+    /*
+     * Finally write out the temporary buffer to the real one if it is not too
+     * small.
+     */
+    if (RT_SUCCESS(rc))
+    {
+        VBoxHGCMParmUInt32Set(&paParms[2], buffer.size());
         /* Copy the memory if it fits into the guest buffer */
-        if (iTmpBuf <= cchBuf)
-            memcpy(pchBuf, TmpBuf.get(), iTmpBuf);
+        if (buffer.size() <= cchBuf)
+            buffer.copy(pchBuf, cchBuf);
         else
             rc = VERR_BUFFER_OVERFLOW;
@@ -753 +797 @@
 void Service::notifyHost(const char *pszProperty)
 {
-    char szValue[MAX_VALUE_LEN];
-    uint64_t u64Timestamp = 0;
-    uint32_t fFlags = NILFLAG;
+#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
     char szFlags[MAX_FLAGS_LEN];
     char *pszName = NULL, *pszValue = NULL, *pszFlags = NULL;
-
-    AssertPtr(mpValueNode);
+    int rc = VINF_SUCCESS;
+
     if (NULL == mpfnHostCallback)
         return;  /* Nothing to do. */
-    int rc = CFGMR3QueryString(mpValueNode, pszProperty, szValue,
-                               sizeof(szValue));
+    PropertyList::const_iterator it;
+    bool found = false;
+    if (RT_SUCCESS(rc))
+        for (it = mProperties.begin(); it != mProperties.end(); ++it)
+            if (it->mName.compare(pszProperty) == 0)
+            {
+                found = true;
+                break;
+            }
     /*
      * First case: if the property exists then send the host its current value
      */
-    if (rc != VERR_CFGM_VALUE_NOT_FOUND)
-    {
-        if (RT_SUCCESS(rc))
-            rc = CFGMR3QueryU64(mpTimestampNode, pszProperty, &u64Timestamp);
-        if (RT_SUCCESS(rc))
-            rc = CFGMR3QueryU32(mpFlagsNode, pszProperty, &fFlags);
-        if (RT_SUCCESS(rc))
-            rc = writeFlags(fFlags, szFlags);
+    if (found)
+    {
+        rc = writeFlags(it->mFlags, szFlags);
         if (RT_SUCCESS(rc))
             rc = RTStrDupEx(&pszName, pszProperty);
         if (RT_SUCCESS(rc))
-            rc = RTStrDupEx(&pszValue, szValue);
+            rc = RTStrDupEx(&pszValue, it->mValue.c_str());
         if (RT_SUCCESS(rc))
             rc = RTStrDupEx(&pszFlags, szFlags);
@@ -785 +829 @@
                              (PFNRT)Service::reqNotify, 7, mpfnHostCallback,
                              mpvHostData, pszName, pszValue,
-                             (uint32_t) RT_HIDWORD(u64Timestamp),
-                             (uint32_t) RT_LODWORD(u64Timestamp), pszFlags);
-        if (RT_FAILURE(rc)) /* clean up */
-        {
-            RTStrFree(pszName);
-            RTStrFree(pszValue);
-            RTStrFree(pszFlags);
-        }
+                             (uint32_t) RT_HIDWORD(it->mTimestamp),
+                             (uint32_t) RT_LODWORD(it->mTimestamp), pszFlags);
     }
     else
@@ -812 +850 @@
         RTStrFree(pszFlags);
     }
+#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
 }
 
@@ -861 +900 @@
                  u32ClientID, eFunction, cParms, paParms));
 
-    switch (eFunction)
-    {
-        /* The guest wishes to read a property */
-        case GET_PROP:
-            LogFlowFunc(("GET_PROP\n"));
-            rc = getProperty(cParms, paParms);
-            break;
-
-        /* The guest wishes to set a property */
-        case SET_PROP:
-            LogFlowFunc(("SET_PROP\n"));
-            rc = setProperty(cParms, paParms, true);
-            break;
-
-        /* The guest wishes to set a property value */
-        case SET_PROP_VALUE:
-            LogFlowFunc(("SET_PROP_VALUE\n"));
-            rc = setProperty(cParms, paParms, true);
-            break;
-
-        /* The guest wishes to remove a configuration value */
-        case DEL_PROP:
-            LogFlowFunc(("DEL_PROP\n"));
-            rc = delProperty(cParms, paParms, true);
-            break;
-
-        /* The guest wishes to enumerate all properties */
-        case ENUM_PROPS:
-            LogFlowFunc(("ENUM_PROPS\n"));
-            rc = enumProps(cParms, paParms);
-            break;
-        default:
-            rc = VERR_NOT_IMPLEMENTED;
+    try
+    {
+        switch (eFunction)
+        {
+            /* The guest wishes to read a property */
+            case GET_PROP:
+                LogFlowFunc(("GET_PROP\n"));
+                rc = getProperty(cParms, paParms);
+                break;
+
+            /* The guest wishes to set a property */
+            case SET_PROP:
+                LogFlowFunc(("SET_PROP\n"));
+                rc = setProperty(cParms, paParms, true);
+                break;
+
+            /* The guest wishes to set a property value */
+            case SET_PROP_VALUE:
+                LogFlowFunc(("SET_PROP_VALUE\n"));
+                rc = setProperty(cParms, paParms, true);
+                break;
+
+            /* The guest wishes to remove a configuration value */
+            case DEL_PROP:
+                LogFlowFunc(("DEL_PROP\n"));
+                rc = delProperty(cParms, paParms, true);
+                break;
+
+            /* The guest wishes to enumerate all properties */
+            case ENUM_PROPS:
+                LogFlowFunc(("ENUM_PROPS\n"));
+                rc = enumProps(cParms, paParms);
+                break;
+
+            default:
+                rc = VERR_NOT_IMPLEMENTED;
+        }
+    }
+    catch (std::bad_alloc)
+    {
+        rc = VERR_NO_MEMORY;
     }
     if (fCallSync)
@@ -915 +962 @@
                  eFunction, cParms, paParms));
 
-    switch (eFunction)
-    {
-        /* Set the root CFGM node used.  This should be called when instantiating
-         * the service. */
-        /* This whole case is due to go away, so I will not clean it up. */
-        case SET_CFGM_NODE:
+    try
+    {
+        switch (eFunction)
         {
-            LogFlowFunc(("SET_CFGM_NODE\n"));
-
-            if (   (cParms != 3)
-                || (paParms[0].type != VBOX_HGCM_SVC_PARM_PTR)   /* pValue */
-                || (paParms[1].type != VBOX_HGCM_SVC_PARM_PTR)   /* pTimestamp */
-                || (paParms[2].type != VBOX_HGCM_SVC_PARM_PTR)   /* pFlags */
-               )
-                rc = VERR_INVALID_PARAMETER;
-            else
-            {
-                PCFGMNODE pValue = NULL, pTimestamp = NULL, pFlags = NULL;
-                uint32_t cbDummy;
-
-                rc = VBoxHGCMParmPtrGet (&paParms[0], (void **) &pValue, &cbDummy);
-                if (RT_SUCCESS(rc))
-                    rc = VBoxHGCMParmPtrGet (&paParms[1], (void **) &pTimestamp, &cbDummy);
-                if (RT_SUCCESS(rc))
-                    rc = VBoxHGCMParmPtrGet (&paParms[2], (void **) &pFlags, &cbDummy);
-                if (RT_SUCCESS(rc))
-                {
-                    mpValueNode     = pValue;
-                    mpTimestampNode = pTimestamp;
-                    mpFlagsNode     = pFlags;
-                }
-            }
-        } break;
-
-        /* The host wishes to read a configuration value */
-        case GET_PROP_HOST:
-            LogFlowFunc(("GET_PROP_HOST\n"));
-            rc = getProperty(cParms, paParms);
-            break;
-
-        /* The host wishes to set a configuration value */
-        case SET_PROP_HOST:
-            LogFlowFunc(("SET_PROP_HOST\n"));
-            rc = setProperty(cParms, paParms, false);
-            break;
-
-        /* The host wishes to set a configuration value */
-        case SET_PROP_VALUE_HOST:
-            LogFlowFunc(("SET_PROP_VALUE_HOST\n"));
-            rc = setProperty(cParms, paParms, false);
-            break;
-
-        /* The host wishes to remove a configuration value */
-        case DEL_PROP_HOST:
-            LogFlowFunc(("DEL_PROP_HOST\n"));
-            rc = delProperty(cParms, paParms, false);
-            break;
-
-        /* The host wishes to enumerate all properties */
-        case ENUM_PROPS_HOST:
-            LogFlowFunc(("ENUM_PROPS\n"));
-            rc = enumProps(cParms, paParms);
-            break;
-        default:
-            rc = VERR_NOT_SUPPORTED;
-            break;
+            /* The host wishes to set a block of properties */
+            case SET_PROPS_HOST:
+                LogFlowFunc(("SET_PROPS_HOST\n"));
+                rc = setPropertyBlock(cParms, paParms);
+                break;
+
+            /* The host wishes to read a configuration value */
+            case GET_PROP_HOST:
+                LogFlowFunc(("GET_PROP_HOST\n"));
+                rc = getProperty(cParms, paParms);
+                break;
+
+            /* The host wishes to set a configuration value */
+            case SET_PROP_HOST:
+                LogFlowFunc(("SET_PROP_HOST\n"));
+                rc = setProperty(cParms, paParms, false);
+                break;
+
+            /* The host wishes to set a configuration value */
+            case SET_PROP_VALUE_HOST:
+                LogFlowFunc(("SET_PROP_VALUE_HOST\n"));
+                rc = setProperty(cParms, paParms, false);
+                break;
+
+            /* The host wishes to remove a configuration value */
+            case DEL_PROP_HOST:
+                LogFlowFunc(("DEL_PROP_HOST\n"));
+                rc = delProperty(cParms, paParms, false);
+                break;
+
+            /* The host wishes to enumerate all properties */
+            case ENUM_PROPS_HOST:
+                LogFlowFunc(("ENUM_PROPS\n"));
+                rc = enumProps(cParms, paParms);
+                break;
+
+            default:
+                rc = VERR_NOT_SUPPORTED;
+                break;
+        }
+    }
+    catch (std::bad_alloc)
+    {
+        rc = VERR_NO_MEMORY;
     }
 
@@ -989 +1018 @@
 int Service::uninit()
 {
-    int rc;
+    int rc = VINF_SUCCESS;
     unsigned count = 0;
 
     mfExitThread = true;
+#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
     rc = RTReqCallEx(mReqQueue, NULL, 0, RTREQFLAGS_NO_WAIT, (PFNRT)reqVoid, 0);
     if (RT_SUCCESS(rc))
@@ -1001 +1031 @@
             Assert(RT_SUCCESS(rc) || ((VERR_TIMEOUT == rc) && (count != 5)));
         } while ((VERR_TIMEOUT == rc) && (count < 300));
+#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
     if (RT_SUCCESS(rc))
         RTReqDestroyQueue(mReqQueue);

trunk/src/VBox/HostServices/GuestProperties/testcase/Makefile.kmk

@@ -29 +29 @@
 PROGRAMS += tstGuestPropSvc
 tstGuestPropSvc_TEMPLATE = VBOXR3TSTEXE
-tstGuestPropSvc_DEFS     = VBOX_WITH_HGCM
-tstGuestPropSvc_SOURCES  = tstGuestPropSvc.cpp
+# The second define here is to ensure that the testcase will run fast, without
+# waiting for any thread synchronisation.
+tstGuestPropSvc_DEFS     = VBOX_WITH_HGCM VBOX_GUEST_PROP_TEST_NOTHREAD
+tstGuestPropSvc_SOURCES  = tstGuestPropSvc.cpp ../service.cpp
 tstGuestPropSvc_LIBS     = $(LIB_RUNTIME)
+
+# Set this in LocalConfig.kmk if you are working on the guest property service
+# to automatically run the testcase at build time.
+ifdef VBOX_RUN_GUEST_PROPERTY_TEST
+ ifndef VBOX_ONLY_SDK
+  TESTING  += $(PATH_tstGuestPropSvc)/tstGuestPropSvc.run
+  OTHERS += $(PATH_tstGuestPropSvc)/tstGuestPropSvc.run
+$$(PATH_tstGuestPropSvc)/tstGuestPropSvc.run: $$(INSTARGET_tstGuestPropSvc)
+        $(INSTARGET_tstGuestPropSvc) quiet
+        $(QUIET)$(APPEND) -t "$@" "done"
+ endif
+endif
 
 endif # VBOX_WITH_TESTCASES

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

@@ -2 +2 @@
 /** @file
  *
- * Testcase for the guest property service.  For now, this only tests
- * flag conversion.
+ * Testcase for the guest property service.
  */
 
@@ -31 +30 @@
 using namespace guestProp;
 
-/**
- * A list of valid flag strings.  The flag conversion functions should accept
- * these and convert them from string to a flag type and back without errors.
+extern "C" DECLCALLBACK(DECLEXPORT(int)) VBoxHGCMSvcLoad (VBOXHGCMSVCFNTABLE *ptable);
+
+/** Set a pointer value to an HGCM parameter structure */
+static void VBoxHGCMParmPtrSet (VBOXHGCMSVCPARM *pParm, void *pv, uint32_t cb)
+{
+    pParm->type = VBOX_HGCM_SVC_PARM_PTR;
+    pParm->u.pointer.addr = pv;
+    pParm->u.pointer.size = cb;
+}
+
+/** Extract a uint64_t value from an HGCM parameter structure */
+static int VBoxHGCMParmUInt32Get (VBOXHGCMSVCPARM *pParm, uint32_t *pu32Value)
+{
+    if (pParm->type == VBOX_HGCM_SVC_PARM_32BIT)
+    {
+        *pu32Value = pParm->u.uint32;
+        return VINF_SUCCESS;
+    }
+
+    return VERR_INVALID_PARAMETER;
+}
+
+/** Extract a uint64_t value from an HGCM parameter structure */
+static int VBoxHGCMParmUInt64Get (VBOXHGCMSVCPARM *pParm, uint64_t *pu64Value)
+{
+    if (pParm->type == VBOX_HGCM_SVC_PARM_64BIT)
+    {
+        *pu64Value = pParm->u.uint64;
+        return VINF_SUCCESS;
+    }
+
+    return VERR_INVALID_PARAMETER;
+}
+
+/** Simple call handle structure for the guest call completion callback */
+struct VBOXHGCMCALLHANDLE_TYPEDEF
+{
+    /** Where to store the result code */
+    int32_t rc;
+};
+
+/** Call completion callback for guest calls. */
+static void callComplete(VBOXHGCMCALLHANDLE callHandle, int32_t rc)
+{
+    callHandle->rc = rc;
+}
+
+/**
+ * Initialise the HGCM service table as much as we need to start the
+ * service
+ * @param  pTable the table to initialise
+ */
+void initTable(VBOXHGCMSVCFNTABLE *pTable, VBOXHGCMSVCHELPERS *pHelpers)
+{
+    pTable->cbSize = sizeof (VBOXHGCMSVCFNTABLE);
+    pTable->u32Version = VBOX_HGCM_SVC_VERSION;
+    pHelpers->pfnCallComplete = callComplete;
+    pTable->pHelpers = pHelpers;
+}
+
+/**
+ * A list of valid flag strings for testConvertFlags.  The flag conversion
+ * functions should accept these and convert them from string to a flag type
+ * and back without errors.
  */
 struct flagStrings
@@ -44 +104 @@
 validFlagStrings[] =
 {
-    { "", "" },
+    { "  ", "" },
     { "transient, ", "TRANSIENT" },
-    { "  rdOnLyHOST, transIENT  ,     READONLY    ", "TRANSIENT, READONLY" }
-};
-
+    { "  rdOnLyHOST, transIENT  ,     READONLY    ", "TRANSIENT, READONLY" },
+    { " rdonlyguest", "RDONLYGUEST" },
+    { "rdonlyhost     ", "RDONLYHOST" }
+};
+
+/**
+ * A list of invalid flag strings for testConvertFlags.  The flag conversion
+ * functions should reject these.
+ */
+const char *invalidFlagStrings[] =
+{
+    "RDONLYHOST,,",
+    "  TRANSIENT READONLY"
+};
+
+/**
+ * Test the flag conversion functions.
+ * @returns iprt status value to indicate whether the test went as expected.
+ * @note    prints its own diagnostic information to stdout.
+ */
 int testConvertFlags()
 {
@@ -55 +132 @@
     for (unsigned i = 0; i < RT_ELEMENTS(validFlagStrings) && RT_SUCCESS(rc); ++i)
     {
-        char szFlagBuffer[MAX_FLAGS_LEN];
+        char szFlagBuffer[MAX_FLAGS_LEN * 2];
         uint32_t fFlags;
         rc = validateFlags(validFlagStrings[i].pcszIn, &fFlags);
         if (RT_FAILURE(rc))
-            RTPrintf("tstGuestPropSvc: FAILURE - Failed to validate flag string %s.\n", validFlagStrings[i].pcszIn);
+            RTPrintf("tstGuestPropSvc: FAILURE - Failed to validate flag string '%s'.\n", validFlagStrings[i].pcszIn);
         if (RT_SUCCESS(rc))
         {
             rc = writeFlags(fFlags, szFlagBuffer);
             if (RT_FAILURE(rc))
-                RTPrintf("tstGuestPropSvc: FAILURE - Failed to convert flag string %s back to a string.\n",
+                RTPrintf("tstGuestPropSvc: FAILURE - Failed to convert flag string '%s' back to a string.\n",
                             validFlagStrings[i].pcszIn);
         }
         if (RT_SUCCESS(rc) && (strlen(szFlagBuffer) > MAX_FLAGS_LEN - 1))
         {
-            RTPrintf("tstGuestPropSvc: FAILURE - String %s converts back to a flag string which is too long.\n",
+            RTPrintf("tstGuestPropSvc: FAILURE - String '%s' converts back to a flag string which is too long.\n",
                         validFlagStrings[i].pcszIn);
             rc = VERR_TOO_MUCH_DATA;
@@ -75 +152 @@
         if (RT_SUCCESS(rc) && (strcmp(szFlagBuffer, validFlagStrings[i].pcszOut) != 0))
         {
-            RTPrintf("tstGuestPropSvc: FAILURE - String %s converts back to %s instead of to %s\n",
+            RTPrintf("tstGuestPropSvc: FAILURE - String '%s' converts back to '%s' instead of to '%s'\n",
                         validFlagStrings[i].pcszIn, szFlagBuffer,
                         validFlagStrings[i].pcszOut);
@@ -81 +158 @@
         }
     }
+    if (RT_SUCCESS(rc))
+    {
+        RTPrintf("Testing rejection of invalid flags strings.\n");
+        for (unsigned i = 0; i < RT_ELEMENTS(invalidFlagStrings) && RT_SUCCESS(rc); ++i)
+        {
+            uint32_t fFlags;
+            /* This is required to fail. */
+            if (RT_SUCCESS(validateFlags(invalidFlagStrings[i], &fFlags)))
+            {
+                RTPrintf("String '%s' was incorrectly accepted as a valid flag string.\n",
+                         invalidFlagStrings[i]);
+                rc = VERR_PARSE_ERROR;
+            }
+        }
+    }
+    if (RT_SUCCESS(rc))
+    {
+        char szFlagBuffer[MAX_FLAGS_LEN * 2];
+        uint32_t u32BadFlags = ALLFLAGS << 1;
+        RTPrintf("Testing rejection of an invalid flags field.\n");
+        /* This is required to fail. */
+        if (RT_SUCCESS(writeFlags(u32BadFlags, szFlagBuffer)))
+        {
+            RTPrintf("Flags 0x%x were incorrectly written out as '%.*s'\n",
+                     u32BadFlags, MAX_FLAGS_LEN, szFlagBuffer);
+            rc = VERR_PARSE_ERROR;
+        }
+    }
     return rc;
 }
 
+/**
+ * List of property names for testSetPropsHost.
+ */
+const char *apcszNameBlock[] =
+{
+    "test/name/",
+    "test name",
+    "TEST NAME",
+    "/test/name",
+    NULL
+};
+
+/**
+ * List of property values for testSetPropsHost.
+ */
+const char *apcszValueBlock[] =
+{
+    "test/value/",
+    "test value",
+    "TEST VALUE",
+    "/test/value",
+    NULL
+};
+
+/**
+ * List of property timestamps for testSetPropsHost.
+ */
+uint64_t au64TimestampBlock[] =
+{
+    0, 999, 999999, 999999999999, 0
+};
+
+/**
+ * List of property flags for testSetPropsHost.
+ */
+const char *apcszFlagsBlock[] =
+{
+    "",
+    "readonly, transient",
+    "RDONLYHOST",
+    "RdOnlyGuest",
+    NULL
+};
+
+/**
+ * Test the SET_PROPS_HOST function.
+ * @returns iprt status value to indicate whether the test went as expected.
+ * @note    prints its own diagnostic information to stdout.
+ */
+int testSetPropsHost(VBOXHGCMSVCFNTABLE *ptable)
+{
+    int rc = VINF_SUCCESS;
+    RTPrintf("Testing the SET_PROPS_HOST call.\n");
+    if (!VALID_PTR(ptable->pfnHostCall))
+    {
+        RTPrintf("Invalid pfnHostCall() pointer\n");
+        rc = VERR_INVALID_POINTER;
+    }
+    if (RT_SUCCESS(rc))
+    {
+        VBOXHGCMSVCPARM paParms[4];
+        VBoxHGCMParmPtrSet(&paParms[0], (void *) apcszNameBlock, 0);
+        VBoxHGCMParmPtrSet(&paParms[1], (void *) apcszValueBlock, 0);
+        VBoxHGCMParmPtrSet(&paParms[2], (void *) au64TimestampBlock, 0);
+        VBoxHGCMParmPtrSet(&paParms[3], (void *) apcszFlagsBlock, 0);
+        rc = ptable->pfnHostCall(ptable->pvService, SET_PROPS_HOST, 4,
+                                 paParms);
+        if (RT_FAILURE(rc))
+            RTPrintf("SET_PROPS_HOST call failed with rc=%Rrc\n", rc);
+    }
+    return rc;
+}
+
+/** Result strings for zeroth enumeration test */
+static const char *pcchEnumResult0[] =
+{
+    "test/name/\0test/value/\0""0\0",
+    "test name\0test value\0""999\0TRANSIENT, READONLY",
+    "TEST NAME\0TEST VALUE\0""999999\0RDONLYHOST",
+    "/test/name\0/test/value\0""999999999999\0RDONLYGUEST",
+    NULL
+};
+
+/** Result string sizes for zeroth enumeration test */
+static const size_t cchEnumResult0[] =
+{
+    sizeof("test/name/\0test/value/\0""0\0"),
+    sizeof("test name\0test value\0""999\0TRANSIENT, READONLY"),
+    sizeof("TEST NAME\0TEST VALUE\0""999999\0RDONLYHOST"),
+    sizeof("/test/name\0/test/value\0""999999999999\0RDONLYGUEST"),
+    0
+};
+
+/**
+ * The size of the buffer returned by the zeroth enumeration test -
+ * the - 1 at the end is because of the hidden zero terminator
+ */
+static const size_t cchEnumBuffer0 =
+sizeof("test/name/\0test/value/\0""0\0\0"
+"test name\0test value\0""999\0TRANSIENT, READONLY\0"
+"TEST NAME\0TEST VALUE\0""999999\0RDONLYHOST\0"
+"/test/name\0/test/value\0""999999999999\0RDONLYGUEST\0\0\0\0\0") - 1;
+
+/** Result strings for first and second enumeration test */
+static const char *pcchEnumResult1[] =
+{
+    "TEST NAME\0TEST VALUE\0""999999\0RDONLYHOST",
+    "/test/name\0/test/value\0""999999999999\0RDONLYGUEST",
+    NULL
+};
+
+/** Result string sizes for first and second enumeration test */
+static const size_t cchEnumResult1[] =
+{
+    sizeof("TEST NAME\0TEST VALUE\0""999999\0RDONLYHOST"),
+    sizeof("/test/name\0/test/value\0""999999999999\0RDONLYGUEST"),
+    0
+};
+
+/**
+ * The size of the buffer returned by the first enumeration test -
+ * the - 1 at the end is because of the hidden zero terminator
+ */
+static const size_t cchEnumBuffer1 =
+sizeof("TEST NAME\0TEST VALUE\0""999999\0RDONLYHOST\0"
+"/test/name\0/test/value\0""999999999999\0RDONLYGUEST\0\0\0\0\0") - 1;
+
+static const struct enumStringStruct
+{
+    /** The enumeration pattern to test */
+    const char *pcszPatterns;
+    /** The size of the pattern string */
+    const size_t cchPatterns;
+    /** The expected enumeration output strings */
+    const char **ppcchResult;
+    /** The size of the output strings */
+    const size_t *pcchResult;
+    /** The size of the buffer needed for the enumeration */
+    const size_t cchBuffer;
+}
+enumStrings[] =
+{
+    {
+        "", sizeof(""),
+        pcchEnumResult0,
+        cchEnumResult0,
+        cchEnumBuffer0
+    },
+    {
+        "/*\0?E*", sizeof("/*\0?E*"),
+        pcchEnumResult1,
+        cchEnumResult1,
+        cchEnumBuffer1
+    },
+    {
+        "/*|?E*", sizeof("/*|?E*"),
+        pcchEnumResult1,
+        cchEnumResult1,
+        cchEnumBuffer1
+    }
+};
+
+/**
+ * Test the ENUM_PROPS_HOST function.
+ * @returns iprt status value to indicate whether the test went as expected.
+ * @note    prints its own diagnostic information to stdout.
+ */
+int testEnumPropsHost(VBOXHGCMSVCFNTABLE *ptable)
+{
+    int rc = VINF_SUCCESS;
+    RTPrintf("Testing the ENUM_PROPS_HOST call.\n");
+    if (!VALID_PTR(ptable->pfnHostCall))
+    {
+        RTPrintf("Invalid pfnHostCall() pointer\n");
+        rc = VERR_INVALID_POINTER;
+    }
+    for (unsigned i = 0; RT_SUCCESS(rc) && i < RT_ELEMENTS(enumStrings);
+         ++i)
+    {
+        char buffer[2048];
+        VBOXHGCMSVCPARM paParms[3];
+        VBoxHGCMParmPtrSet(&paParms[0],
+                           (void *) enumStrings[i].pcszPatterns,
+                           enumStrings[i].cchPatterns);
+        VBoxHGCMParmPtrSet(&paParms[1],
+                           (void *) buffer,
+                           enumStrings[i].cchBuffer - 1);
+        AssertBreakStmt(sizeof(buffer) > enumStrings[i].cchBuffer,
+                        rc = VERR_INTERNAL_ERROR);
+        if (RT_SUCCESS(rc))
+        {
+            /* This should fail as the buffer is too small. */
+            int rc2 = ptable->pfnHostCall(ptable->pvService, ENUM_PROPS_HOST,
+                                          3, paParms);
+            if (rc2 != VERR_BUFFER_OVERFLOW)
+            {
+                RTPrintf("ENUM_PROPS_HOST returned %Rrc instead of VERR_BUFFER_OVERFLOW on too small buffer, pattern number %d\n", rc2, i);
+                rc = VERR_BUFFER_OVERFLOW;
+            }
+            else
+            {
+                uint32_t cchBufferActual;
+                rc = VBoxHGCMParmUInt32Get(&paParms[2], &cchBufferActual);
+                if (RT_SUCCESS(rc) && cchBufferActual != enumStrings[i].cchBuffer)
+                {
+                    RTPrintf("ENUM_PROPS_HOST requested a buffer size of %lu instead of %lu for pattern number %d\n", cchBufferActual, enumStrings[i].cchBuffer, i);
+                    rc = VERR_OUT_OF_RANGE;
+                }
+                else if (RT_FAILURE(rc))
+                    RTPrintf("ENUM_PROPS_HOST did not return the required buffer size properly for pattern %d\n", i);
+            }
+        }
+        if (RT_SUCCESS(rc))
+        {
+            VBoxHGCMParmPtrSet(&paParms[1], (void *) buffer,
+                               enumStrings[i].cchBuffer);
+            rc = ptable->pfnHostCall(ptable->pvService, ENUM_PROPS_HOST,
+                                      3, paParms);
+            if (RT_FAILURE(rc))
+                RTPrintf("ENUM_PROPS_HOST call failed for pattern %d with rc=%Rrc\n", i, rc);
+            else
+                /* Look for each of the result strings in the buffer which was returned */
+                for (unsigned j = 0; RT_SUCCESS(rc) && enumStrings[i].ppcchResult[j] != NULL;
+                     ++j)
+                {
+                    bool found = false;
+                    for (unsigned k = 0; !found && k <   enumStrings[i].cchBuffer
+                                                       - enumStrings[i].pcchResult[j];
+                         ++k)
+                        if (memcmp(buffer + k, enumStrings[i].ppcchResult[j],
+                            enumStrings[i].pcchResult[j]) == 0)
+                            found = true;
+                    if (!found)
+                    {
+                        RTPrintf("ENUM_PROPS_HOST did not produce the expected output for pattern %d\n",
+                                 i);
+                        rc = VERR_UNRESOLVED_ERROR;
+                    }
+                }
+        }
+    }
+    return rc;
+}
+
+/** Array of properties for testing SET_PROP_HOST and _GUEST. */
+static const struct
+{
+    /** Property name */
+    const char *pcszName;
+    /** Property value */
+    const char *pcszValue;
+    /** Property flags */
+    const char *pcszFlags;
+    /** Should this be set as the host or the guest? */
+    bool isHost;
+    /** Should we use SET_PROP or SET_PROP_VALUE? */
+    bool useSetProp;
+    /** Should this succeed or be rejected with VERR_PERMISSION_DENIED? */
+    bool isAllowed;
+}
+setProperties[] =
+{
+    { "Red", "Stop!", "transient", false, true, true },
+    { "Amber", "Caution!", "", false, false, true },
+    { "Green", "Go!", "readonly", true, true, true },
+    { "Blue", "What on earth...?", "", true, false, true },
+    { "/test/name", "test", "", false, true, false },
+    { "TEST NAME", "test", "", true, true, false },
+    { "Green", "gone out...", "", false, false, false },
+    { "Green", "gone out...", "", true, false, false },
+    { NULL, NULL, NULL, false, false, false }
+};
+
+/**
+ * Test the SET_PROP, SET_PROP_VALUE, SET_PROP_HOST and SET_PROP_VALUE_HOST
+ * functions.
+ * @returns iprt status value to indicate whether the test went as expected.
+ * @note    prints its own diagnostic information to stdout.
+ */
+int testSetProp(VBOXHGCMSVCFNTABLE *pTable)
+{
+    int rc = VINF_SUCCESS;
+    VBOXHGCMCALLHANDLE_TYPEDEF callHandle = { VINF_SUCCESS };
+    RTPrintf("Testing the SET_PROP, SET_PROP_VALUE, SET_PROP_HOST and SET_PROP_VALUE_HOST calls.\n");
+    for (unsigned i = 0; RT_SUCCESS(rc) && (setProperties[i].pcszName != NULL);
+         ++i)
+    {
+        int command = SET_PROP_VALUE;
+        if (setProperties[i].isHost)
+        {
+            if (setProperties[i].useSetProp)
+                command = SET_PROP_HOST;
+            else
+                command = SET_PROP_VALUE_HOST;
+        }
+        else if (setProperties[i].useSetProp)
+            command = SET_PROP;
+        VBOXHGCMSVCPARM paParms[3];
+        /* Work around silly constant issues - we ought to allow passing
+         * constant strings in the hgcm parameters. */
+        char szName[MAX_NAME_LEN] = "";
+        char szValue[MAX_VALUE_LEN] = "";
+        char szFlags[MAX_FLAGS_LEN] = "";
+        strncat(szName, setProperties[i].pcszName, sizeof(szName));
+        strncat(szValue, setProperties[i].pcszValue, sizeof(szValue));
+        strncat(szFlags, setProperties[i].pcszFlags, sizeof(szFlags));
+        VBoxHGCMParmPtrSet(&paParms[0], szName, strlen(szName) + 1);
+        VBoxHGCMParmPtrSet(&paParms[1], szValue, strlen(szValue) + 1);
+        VBoxHGCMParmPtrSet(&paParms[2], szFlags, strlen(szFlags) + 1);
+        if (setProperties[i].isHost)
+            callHandle.rc = pTable->pfnHostCall(pTable->pvService, command,
+                                                setProperties[i].useSetProp
+                                                    ? 3 : 2, paParms);
+        else
+            pTable->pfnCall(pTable->pvService, &callHandle, 0, NULL, command,
+                            setProperties[i].useSetProp ? 3 : 2, paParms);
+        if (setProperties[i].isAllowed && RT_FAILURE(callHandle.rc))
+        {
+            RTPrintf("Setting property '%s' failed with rc=%Rrc.\n",
+                     setProperties[i].pcszName, callHandle.rc);
+            rc = callHandle.rc;
+        }
+        else if (   !setProperties[i].isAllowed
+                 && (callHandle.rc != VERR_PERMISSION_DENIED)
+                )
+        {
+            RTPrintf("Setting property '%s' returned %Rrc instead of VERR_PERMISSION_DENIED.\n",
+                     setProperties[i].pcszName, callHandle.rc);
+            rc = VERR_UNRESOLVED_ERROR;
+        }
+    }
+    return rc;
+}
+
+/** Array of properties for testing DEL_PROP_HOST and _GUEST. */
+static const struct
+{
+    /** Property name */
+    const char *pcszName;
+    /** Should this be set as the host or the guest? */
+    bool isHost;
+    /** Should this succeed or be rejected with VERR_PERMISSION_DENIED? */
+    bool isAllowed;
+}
+delProperties[] =
+{
+    { "Red", false, true },
+    { "Amber", true, true },
+    { "Red2", false, true },
+    { "Amber2", true, true },
+    { "Green", false, false },
+    { "Green", true, false },
+    { "/test/name", false, false },
+    { "TEST NAME", true, false },
+    { NULL, false, false }
+};
+
+/**
+ * Test the DEL_PROP, and DEL_PROP_HOST functions.
+ * @returns iprt status value to indicate whether the test went as expected.
+ * @note    prints its own diagnostic information to stdout.
+ */
+int testDelProp(VBOXHGCMSVCFNTABLE *pTable)
+{
+    int rc = VINF_SUCCESS;
+    VBOXHGCMCALLHANDLE_TYPEDEF callHandle = { VINF_SUCCESS };
+    RTPrintf("Testing the DEL_PROP and DEL_PROP_HOST calls.\n");
+    for (unsigned i = 0; RT_SUCCESS(rc) && (delProperties[i].pcszName != NULL);
+         ++i)
+    {
+        int command = DEL_PROP;
+        if (delProperties[i].isHost)
+            command = DEL_PROP_HOST;
+        VBOXHGCMSVCPARM paParms[1];
+        /* Work around silly constant issues - we ought to allow passing
+         * constant strings in the hgcm parameters. */
+        char szName[MAX_NAME_LEN] = "";
+        strncat(szName, delProperties[i].pcszName, sizeof(szName));
+        VBoxHGCMParmPtrSet(&paParms[0], szName, strlen(szName) + 1);
+        if (delProperties[i].isHost)
+            callHandle.rc = pTable->pfnHostCall(pTable->pvService, command,
+                                                1, paParms);
+        else
+            pTable->pfnCall(pTable->pvService, &callHandle, 0, NULL, command,
+                            1, paParms);
+        if (delProperties[i].isAllowed && RT_FAILURE(callHandle.rc))
+        {
+            RTPrintf("Deleting property '%s' failed with rc=%Rrc.\n",
+                     delProperties[i].pcszName, callHandle.rc);
+            rc = callHandle.rc;
+        }
+        else if (   !delProperties[i].isAllowed
+                 && (callHandle.rc != VERR_PERMISSION_DENIED)
+                )
+        {
+            RTPrintf("Deleting property '%s' returned %Rrc instead of VERR_PERMISSION_DENIED.\n",
+                     delProperties[i].pcszName, callHandle.rc);
+            rc = VERR_UNRESOLVED_ERROR;
+        }
+    }
+    return rc;
+}
+
+/** Array of properties for testing GET_PROP_HOST. */
+static const struct
+{
+    /** Property name */
+    const char *pcszName;
+    /** What value/flags pattern do we expect back? */
+    const char *pcchValue;
+    /** What size should the value/flags array be? */
+    uint32_t cchValue;
+    /** Should this proeprty exist? */
+    bool exists;
+    /** Do we expect a particular timestamp? */
+    bool hasTimestamp;
+    /** What timestamp if any do ex expect? */
+    uint64_t u64Timestamp;
+}
+getProperties[] =
+{
+    { "test/name/", "test/value/\0", sizeof("test/value/\0"), true, true, 0 },
+    { "test name", "test value\0TRANSIENT, READONLY",
+      sizeof("test value\0TRANSIENT, READONLY"), true, true, 999 },
+    { "TEST NAME", "TEST VALUE\0RDONLYHOST", sizeof("TEST VALUE\0RDONLYHOST"),
+      true, true, 999999 },
+    { "/test/name", "/test/value\0RDONLYGUEST",
+      sizeof("/test/value\0RDONLYGUEST"), true, true, 999999999999 },
+    { "Green", "Go!\0READONLY", sizeof("Go!\0READONLY"), true, false, 0 },
+    { "Blue", "What on earth...?\0", sizeof("What on earth...?\0"), true,
+      false, 0 },
+    { "Red", "", 0, false, false, 0 },
+    { NULL, NULL, 0, false, false, 0 }
+};
+
+/**
+ * Test the GET_PROP_HOST function.
+ * @returns iprt status value to indicate whether the test went as expected.
+ * @note    prints its own diagnostic information to stdout.
+ */
+int testGetProp(VBOXHGCMSVCFNTABLE *pTable)
+{
+    int rc = VINF_SUCCESS, rc2 = VINF_SUCCESS;
+    RTPrintf("Testing the GET_PROP_HOST call.\n");
+    for (unsigned i = 0; RT_SUCCESS(rc) && (getProperties[i].pcszName != NULL);
+         ++i)
+    {
+        VBOXHGCMSVCPARM paParms[4];
+        /* Work around silly constant issues - we ought to allow passing
+         * constant strings in the hgcm parameters. */
+        char szName[MAX_NAME_LEN] = "";
+        char szBuffer[MAX_VALUE_LEN + MAX_FLAGS_LEN];
+        AssertBreakStmt(sizeof(szBuffer) >= getProperties[i].cchValue,
+                        rc = VERR_INTERNAL_ERROR);
+        strncat(szName, getProperties[i].pcszName, sizeof(szName));
+        VBoxHGCMParmPtrSet(&paParms[0], szName, strlen(szName) + 1);
+        VBoxHGCMParmPtrSet(&paParms[1], szBuffer, sizeof(szBuffer));
+        rc2 = pTable->pfnHostCall(pTable->pvService, GET_PROP_HOST, 4,
+                                  paParms);
+        if (getProperties[i].exists && RT_FAILURE(rc2))
+        {
+            RTPrintf("Getting property '%s' failed with rc=%Rrc.\n",
+                     getProperties[i].pcszName, rc2);
+            rc = rc2;
+        }
+        else if (!getProperties[i].exists && (rc2 != VERR_NOT_FOUND))
+        {
+            RTPrintf("Getting property '%s' returned %Rrc instead of VERR_NOT_FOUND.\n",
+                     getProperties[i].pcszName, rc2);
+            rc = VERR_UNRESOLVED_ERROR;
+        }
+        if (RT_SUCCESS(rc) && getProperties[i].exists)
+        {
+            uint32_t u32ValueLen;
+            rc = VBoxHGCMParmUInt32Get(&paParms[3], &u32ValueLen);
+            if (RT_FAILURE(rc))
+                RTPrintf("Failed to get the size of the output buffer for property '%s'\n",
+                         getProperties[i].pcszName);
+            if (   RT_SUCCESS(rc)
+                && (memcmp(szBuffer, getProperties[i].pcchValue,
+                           getProperties[i].cchValue) != 0)
+               )
+            {
+                RTPrintf("Unexpected result '%.*s' for property '%s', expected '%.*s'.\n",
+                         u32ValueLen, szBuffer, getProperties[i].pcszName,
+                         getProperties[i].cchValue, getProperties[i].pcchValue);
+                rc = VERR_UNRESOLVED_ERROR;
+            }
+            if (RT_SUCCESS(rc) && getProperties[i].hasTimestamp)
+            {
+                uint64_t u64Timestamp;
+                rc = VBoxHGCMParmUInt64Get(&paParms[2], &u64Timestamp);
+                if (RT_FAILURE(rc))
+                    RTPrintf("Failed to get the timestamp for property '%s'\n",
+                             getProperties[i].pcszName);
+                if (   RT_SUCCESS(rc)
+                    && (u64Timestamp != getProperties[i].u64Timestamp)
+                   )
+                {
+                    RTPrintf("Bad timestamp %llu for property '%s', expected %llu.\n",
+                             u64Timestamp, getProperties[i].pcszName,
+                             getProperties[i].u64Timestamp);
+                    rc = VERR_UNRESOLVED_ERROR;
+                }
+            }
+        }
+    }
+    return rc;
+}
+
 int main(int argc, char **argv)
 {
+    VBOXHGCMSVCFNTABLE svcTable;
+    VBOXHGCMSVCHELPERS svcHelpers;
+    initTable(&svcTable, &svcHelpers);
     RTR3Init();
     if (RT_FAILURE(testConvertFlags()))
         return 1;
+    /* The function is inside the service, not HGCM. */
+    if (RT_FAILURE(VBoxHGCMSvcLoad(&svcTable)))
+    {
+        RTPrintf("Failed to start HGCM service.\n");
+        return 1;
+    }
+    if (RT_FAILURE(testSetPropsHost(&svcTable)))
+        return 1;
+    if (RT_FAILURE(testEnumPropsHost(&svcTable)))
+        return 1;
+    if (RT_FAILURE(testSetProp(&svcTable)))
+        return 1;
+    if (RT_FAILURE(testDelProp(&svcTable)))
+        return 1;
+    if (RT_FAILURE(testGetProp(&svcTable)))
+        return 1;
     RTPrintf("tstGuestPropSvc: SUCCEEDED.\n");
     return 0;
 }
-

trunk/src/VBox/Main/ConsoleImpl.cpp

@@ -96 +96 @@
 #include <algorithm>
 #include <memory> // for auto_ptr
+#include <vector>
 
 
@@ -1062 +1063 @@
             rc = VERR_UNRESOLVED_ERROR;  /** @todo translate error code */
     return rc;
+}
+
+HRESULT Console::doEnumerateGuestProperties (INPTR BSTR aPatterns,
+                                             ComSafeArrayOut(BSTR, aNames),
+                                             ComSafeArrayOut(BSTR, aValues),
+                                             ComSafeArrayOut(ULONG64, aTimestamps),
+                                             ComSafeArrayOut(BSTR, aFlags))
+{
+    using namespace guestProp;
+
+    VBOXHGCMSVCPARM parm[3];
+
+    Utf8Str utf8Patterns(aPatterns);
+    parm[0].type = VBOX_HGCM_SVC_PARM_PTR;
+    parm[0].u.pointer.addr = utf8Patterns.mutableRaw();
+    parm[0].u.pointer.size = utf8Patterns.length() + 1;
+
+    /*
+     * Now things get slightly complicated.  Due to a race with the guest adding
+     * properties, there is no good way to know how much large a buffer to provide
+     * the service to enumerate into.  We choose a decent starting size and loop a
+     * few times, each time retrying with the size suggested by the service plus
+     * one Kb.
+     */
+    size_t cchBuf = 4096;
+    Utf8Str Utf8Buf;
+    int vrc = VERR_BUFFER_OVERFLOW;
+    for (unsigned i = 0; i < 10 && (VERR_BUFFER_OVERFLOW == vrc); ++i)
+    {
+        Utf8Buf.alloc(cchBuf + 1024);
+        if (Utf8Buf.isNull())
+            return E_OUTOFMEMORY;
+        parm[1].type = VBOX_HGCM_SVC_PARM_PTR;
+        parm[1].u.pointer.addr = Utf8Buf.mutableRaw();
+        parm[1].u.pointer.size = cchBuf + 1024;
+        vrc = mVMMDev->hgcmHostCall ("VBoxGuestPropSvc", ENUM_PROPS_HOST, 3,
+                                     &parm[0]);
+        if (parm[2].type != VBOX_HGCM_SVC_PARM_32BIT)
+            return setError (E_FAIL, tr ("Internal application error"));
+        cchBuf = parm[2].u.uint32;
+    }
+    if (VERR_BUFFER_OVERFLOW == vrc)
+        return setError (E_UNEXPECTED, tr ("Temporary failure due to guest activity, please retry"));
+
+    /*
+     * Finally we have to unpack the data returned by the service into the safe
+     * arrays supplied by the caller.  We start by counting the number of entries.
+     */
+    const char *pszBuf
+        = reinterpret_cast<const char *>(parm[1].u.pointer.addr);
+    unsigned cEntries = 0;
+    /* The list is terminated by a zero-length string at the end of a set
+     * of four strings. */
+    for (size_t i = 0; strlen(pszBuf + i) != 0; )
+    {
+       /* We are counting sets of four strings. */
+       for (unsigned j = 0; j < 4; ++j)
+           i += strlen(pszBuf + i) + 1;
+       ++cEntries;
+    }
+
+    /*
+     * And now we create the COM safe arrays and fill them in.
+     */
+    com::SafeArray <BSTR> names(cEntries);
+    com::SafeArray <BSTR> values(cEntries);
+    com::SafeArray <ULONG64> timestamps(cEntries);
+    com::SafeArray <BSTR> flags(cEntries);
+    size_t iBuf = 0;
+    /* Rely on the service to have formated the data correctly. */
+    for (unsigned i = 0; i < cEntries; ++i)
+    {
+        size_t cchName = strlen(pszBuf + iBuf);
+        Bstr(pszBuf + iBuf).detachTo(&names[i]);
+        iBuf += cchName + 1;
+        size_t cchValue = strlen(pszBuf + iBuf);
+        Bstr(pszBuf + iBuf).detachTo(&values[i]);
+        iBuf += cchValue + 1;
+        size_t cchTimestamp = strlen(pszBuf + iBuf);
+        timestamps[i] = RTStrToUInt64(pszBuf + iBuf);
+        iBuf += cchTimestamp + 1;
+        size_t cchFlags = strlen(pszBuf + iBuf);
+        Bstr(pszBuf + iBuf).detachTo(&flags[i]);
+        iBuf += cchFlags + 1;
+    }
+    names.detachTo(ComSafeArrayOutArg (aNames));
+    values.detachTo(ComSafeArrayOutArg (aValues));
+    timestamps.detachTo(ComSafeArrayOutArg (aTimestamps));
+    flags.detachTo(ComSafeArrayOutArg (aFlags));
+    return S_OK;
 }
 #endif
@@ -3687 +3778 @@
      * autoVMCaller, so there is no need to hold a lock of this */
 
-    using namespace guestProp;
-
-    VBOXHGCMSVCPARM parm[3];
-
-    Utf8Str utf8Patterns(aPatterns);
-    parm[0].type = VBOX_HGCM_SVC_PARM_PTR;
-    parm[0].u.pointer.addr = utf8Patterns.mutableRaw();
-    parm[0].u.pointer.size = utf8Patterns.length() + 1;
-
-    /*
-     * Now things get slightly complicated.  Due to a race with the guest adding
-     * properties, there is no good way to know how much large a buffer to provide
-     * the service to enumerate into.  We choose a decent starting size and loop a
-     * few times, each time retrying with the size suggested by the service plus
-     * one Kb.
-     */
-    size_t cchBuf = 4096;
-    Utf8Str Utf8Buf;
-    int vrc = VERR_BUFFER_OVERFLOW;
-    for (unsigned i = 0; i < 10 && (VERR_BUFFER_OVERFLOW == vrc); ++i)
-    {
-        Utf8Buf.alloc(cchBuf + 1024);
-        if (Utf8Buf.isNull())
-            return E_OUTOFMEMORY;
-        parm[1].type = VBOX_HGCM_SVC_PARM_PTR;
-        parm[1].u.pointer.addr = Utf8Buf.mutableRaw();
-        parm[1].u.pointer.size = cchBuf + 1024;
-        vrc = mVMMDev->hgcmHostCall ("VBoxGuestPropSvc", ENUM_PROPS_HOST, 3,
-                                     &parm[0]);
-        if (parm[2].type != VBOX_HGCM_SVC_PARM_32BIT)
-            return setError (E_FAIL, tr ("Internal application error"));
-        cchBuf = parm[2].u.uint32;
-    }
-    if (VERR_BUFFER_OVERFLOW == vrc)
-        return setError (E_UNEXPECTED, tr ("Temporary failure due to guest activity, please retry"));
-
-    /*
-     * Finally we have to unpack the data returned by the service into the safe
-     * arrays supplied by the caller.  We start by counting the number of entries.
-     */
-    const char *pszBuf
-        = reinterpret_cast<const char *>(parm[1].u.pointer.addr);
-    unsigned cEntries = 0;
-    /* The list is terminated by a zero-length string at the end of a set
-     * of four strings. */
-    for (size_t i = 0; strlen(pszBuf + i) != 0; )
-    {
-       /* We are counting sets of four strings. */
-       for (unsigned j = 0; j < 4; ++j)
-           i += strlen(pszBuf + i) + 1;
-       ++cEntries;
-    }
-
-    /*
-     * And now we create the COM safe arrays and fill them in.
-     */
-    com::SafeArray <BSTR> names(cEntries);
-    com::SafeArray <BSTR> values(cEntries);
-    com::SafeArray <ULONG64> timestamps(cEntries);
-    com::SafeArray <BSTR> flags(cEntries);
-    size_t iBuf = 0;
-    /* Rely on the service to have formated the data correctly. */
-    for (unsigned i = 0; i < cEntries; ++i)
-    {
-        size_t cchName = strlen(pszBuf + iBuf);
-        Bstr(pszBuf + iBuf).detachTo(&names[i]);
-        iBuf += cchName + 1;
-        size_t cchValue = strlen(pszBuf + iBuf);
-        Bstr(pszBuf + iBuf).detachTo(&values[i]);
-        iBuf += cchValue + 1;
-        size_t cchTimestamp = strlen(pszBuf + iBuf);
-        timestamps[i] = RTStrToUInt64(pszBuf + iBuf);
-        iBuf += cchTimestamp + 1;
-        size_t cchFlags = strlen(pszBuf + iBuf);
-        Bstr(pszBuf + iBuf).detachTo(&flags[i]);
-        iBuf += cchFlags + 1;
-    }
-    names.detachTo(ComSafeArrayOutArg (aNames));
-    values.detachTo(ComSafeArrayOutArg (aValues));
-    timestamps.detachTo(ComSafeArrayOutArg (aTimestamps));
-    flags.detachTo(ComSafeArrayOutArg (aFlags));
-    return S_OK;
+    return doEnumerateGuestProperties (aPatterns, ComSafeArrayOutArg(aNames),
+                                       ComSafeArrayOutArg(aValues),
+                                       ComSafeArrayOutArg(aTimestamps),
+                                       ComSafeArrayOutArg(aFlags));
 #endif /* else !defined (VBOX_WITH_GUEST_PROPS) */
 }
@@ -4633 +4646 @@
 #ifdef VBOX_WITH_HGCM
 
+# ifdef VBOX_WITH_GUEST_PROPS
+
+    /* Save all guest property store entries to the machine XML file */
+    com::SafeArray <BSTR> namesOut;
+    com::SafeArray <BSTR> valuesOut;
+    com::SafeArray <ULONG64> timestampsOut;
+    com::SafeArray <BSTR> flagsOut;
+    Bstr pattern("");
+    if (pattern.isNull())
+        rc = E_OUTOFMEMORY;
+    else
+        rc = doEnumerateGuestProperties (Bstr (""), ComSafeArrayAsOutParam (namesOut),
+                                        ComSafeArrayAsOutParam (valuesOut),
+                                        ComSafeArrayAsOutParam (timestampsOut),
+                                        ComSafeArrayAsOutParam (flagsOut));
+    if (SUCCEEDED(rc))
+    {
+        try
+        {
+            std::vector <BSTR> names;
+            std::vector <BSTR> values;
+            std::vector <ULONG64> timestamps;
+            std::vector <BSTR> flags;
+            for (unsigned i = 0; i < namesOut.size(); ++i)
+            {
+                uint32_t fFlags;
+                guestProp::validateFlags (Utf8Str(flagsOut[i]).raw(), &fFlags);
+                if (   !( fFlags & guestProp::TRANSIENT)
+                    || (mMachineState == MachineState_Saving)
+                  )
+                {
+                    names.push_back(namesOut[i]);
+                    values.push_back(valuesOut[i]);
+                    timestamps.push_back(timestampsOut[i]);
+                    flags.push_back(flagsOut[i]);
+                }
+            }
+            com::SafeArray <BSTR> namesIn (names);
+            com::SafeArray <BSTR> valuesIn (values);
+            com::SafeArray <ULONG64> timestampsIn (timestamps);
+            com::SafeArray <BSTR> flagsIn (flags);
+            if (   namesIn.isNull()
+                || valuesIn.isNull()
+                || timestampsIn.isNull()
+                || flagsIn.isNull()
+                )
+                throw std::bad_alloc();
+            /* PushGuestProperties() calls DiscardSettings(), which calls us back */
+            alock.leave();
+            mControl->PushGuestProperties (ComSafeArrayAsInParam (namesIn),
+                                          ComSafeArrayAsInParam (valuesIn),
+                                          ComSafeArrayAsInParam (timestampsIn),
+                                          ComSafeArrayAsInParam (flagsIn));
+            alock.enter();
+        }
+        catch (std::bad_alloc)
+        {
+            rc = E_OUTOFMEMORY;
+        }
+    }
+
+    /* advance percent count */
+    if (aProgress)
+        aProgress->notifyProgress (99 * (++ step) / StepCount );
+
+# endif /* VBOX_WITH_GUEST_PROPS defined */
+
     /* Shutdown HGCM services before stopping the guest, because they might
      * need a cleanup. */
@@ -4650 +4730 @@
     if (aProgress)
         aProgress->notifyProgress (99 * (++ step) / StepCount );
-
-# ifdef VBOX_WITH_GUEST_PROPS
-
-    /* Save all guest property store entries to the machine XML file */
-    PCFGMNODE pValues = CFGMR3GetChild (CFGMR3GetRoot (mpVM), "GuestProps/Values/");
-    PCFGMNODE pTimestamps = CFGMR3GetChild (CFGMR3GetRoot (mpVM), "GuestProps/Timestamps/");
-    PCFGMNODE pFlags = CFGMR3GetChild (CFGMR3GetRoot (mpVM), "GuestProps/Flags/");
-
-    /* Count the number of entries we have */
-    unsigned cValues = 0;
-    PCFGMLEAF pValue;
-    for (pValue = CFGMR3GetFirstValue (pValues); pValue != NULL;
-         pValue = CFGMR3GetNextValue (pValue))
-    {
-        char szPropName[guestProp::MAX_NAME_LEN];
-        vrc = CFGMR3GetValueName (pValue, szPropName, sizeof(szPropName));
-        if (RT_SUCCESS(vrc))
-        {
-            /* Do not send transient properties unless we are saving state */
-            uint32_t fFlags = guestProp::NILFLAG;
-            CFGMR3QueryU32 (pFlags, szPropName, &fFlags);
-            if (!(fFlags & guestProp::TRANSIENT) ||
-                (mMachineState == MachineState_Saving))
-                ++cValues;
-        }
-    }
-
-    /* And pack them into safe arrays */
-    com::SafeArray <BSTR> names(cValues);
-    com::SafeArray <BSTR> values(cValues);
-    com::SafeArray <ULONG64> timestamps(cValues);
-    com::SafeArray <BSTR> flags(cValues);
-    pValue = CFGMR3GetFirstValue (pValues);
-
-    vrc = VINF_SUCCESS;
-    unsigned iProp = 0;
-    while (pValue != NULL && RT_SUCCESS (vrc))
-    {
-        using namespace guestProp;
-
-        char szPropName [MAX_NAME_LEN];
-        char szPropValue [MAX_VALUE_LEN];
-        char szPropFlags [MAX_FLAGS_LEN];
-        ULONG64 u64Timestamp = 0;  /* default */
-        vrc = CFGMR3GetValueName (pValue, szPropName, sizeof (szPropName));
-        if (RT_SUCCESS(vrc))
-            vrc = CFGMR3QueryString (pValues, szPropName, szPropValue,
-                                     sizeof (szPropValue));
-        if (RT_SUCCESS(vrc))
-        {
-            uint32_t fFlags = NILFLAG;
-            CFGMR3QueryU32 (pFlags, szPropName, &fFlags);
-            /* Skip transient properties unless we are saving state */
-            if (!(fFlags & TRANSIENT) ||
-                (mMachineState == MachineState_Saving))
-            {
-                writeFlags(fFlags, szPropFlags);
-                CFGMR3QueryU64 (pTimestamps, szPropName, &u64Timestamp);
-                Bstr(szPropName).cloneTo(&names[iProp]);
-                Bstr(szPropValue).cloneTo(&values[iProp]);
-                timestamps[iProp] = u64Timestamp;
-                Bstr(szPropFlags).cloneTo(&flags[iProp]);
-                ++iProp;
-                if (iProp >= cValues)
-                    vrc = VERR_TOO_MUCH_DATA;
-            }
-            pValue = CFGMR3GetNextValue (pValue);
-        }
-    }
-
-    if (RT_SUCCESS(vrc) || (VERR_TOO_MUCH_DATA == vrc))
-    {
-        /* PushGuestProperties() calls DiscardSettings(), which calls us back */
-        alock.leave();
-        mControl->PushGuestProperties (ComSafeArrayAsInParam (names),
-                                       ComSafeArrayAsInParam (values),
-                                       ComSafeArrayAsInParam (timestamps),
-                                       ComSafeArrayAsInParam (flags));
-        alock.enter();
-    }
-
-    /* advance percent count */
-    if (aProgress)
-        aProgress->notifyProgress (99 * (++ step) / StepCount );
-
-# endif /* VBOX_WITH_GUEST_PROPS defined */
 
 #endif /* VBOX_WITH_HGCM */

trunk/src/VBox/Main/ConsoleImpl2.cpp

@@ -51 +51 @@
 # include <hgcm/HGCM.h> /** @todo it should be possible to register a service
                           * extension using a VMMDev callback. */
+# include <vector>
 #endif /* VBOX_WITH_GUEST_PROPS */
 #include <VBox/intnet.h>
@@ -269 +270 @@
     PCFGMNODE pSataInst = NULL;     /* /Devices/ahci/0/ */
         PCFGMNODE pBiosCfg = NULL;      /* /Devices/pcbios/0/Config/ */
-#ifdef VBOX_WITH_GUEST_PROPS
-    PCFGMNODE pGuestProps = NULL;   /* /GuestProps */
-    PCFGMNODE pValues = NULL;       /* /GuestProps/Values */
-    PCFGMNODE pTimestamps = NULL;   /* /GuestProps/Timestamps */
-    PCFGMNODE pFlags = NULL;        /* /GuestProps/Flags */
-#endif /* VBOX_WITH_GUEST_PROPS defined */
 
     rc = CFGMR3InsertNode(pRoot, "Devices", &pDevices);                             RC_CHECK();
@@ -1729 +1724 @@
 #ifdef VBOX_WITH_GUEST_PROPS
     /*
-     * Shared information services
+     * Guest property service
      */
     {
@@ -1743 +1738 @@
         else
         {
-            rc = CFGMR3InsertNode(pRoot,       "GuestProps", &pGuestProps);             RC_CHECK();
-            rc = CFGMR3InsertNode(pGuestProps, "Values", &pValues);                     RC_CHECK();
-            rc = CFGMR3InsertNode(pGuestProps, "Timestamps", &pTimestamps);             RC_CHECK();
-            rc = CFGMR3InsertNode(pGuestProps, "Flags", &pFlags);                       RC_CHECK();
-
             /* Pull over the properties from the server. */
-            SafeArray <BSTR> names;
-            SafeArray <BSTR> values;
-            SafeArray <ULONG64> timestamps;
-            SafeArray <BSTR> flags;
-            hrc = pConsole->mControl->PullGuestProperties(ComSafeArrayAsOutParam(names),
-                                                ComSafeArrayAsOutParam(values),
-                                                ComSafeArrayAsOutParam(timestamps),
-                                                ComSafeArrayAsOutParam(flags));         H();
-            size_t cProps = names.size();
-            for (size_t i = 0; i < cProps; ++i)
-            {
-                rc = CFGMR3InsertString(pValues, Utf8Str(names[i]).raw(), Utf8Str(values[i]).raw());    RC_CHECK();
-                rc = CFGMR3InsertInteger(pTimestamps, Utf8Str(names[i]).raw(), timestamps[i]);          RC_CHECK();
-                uint32_t fFlags;
-                rc = guestProp::validateFlags(Utf8Str(flags[i]).raw(), &fFlags);
-                AssertLogRelRCReturn(rc, VMSetError(pVM, VERR_INVALID_PARAMETER, RT_SRC_POS,
-                    N_("Guest property '%lS' has invalid flags '%lS' in machine definition file"),
-                       names[i], flags[i]));
-                rc = CFGMR3InsertInteger(pFlags, Utf8Str(names[i]).raw(), fFlags);                      RC_CHECK();
-            }
+            SafeArray <BSTR> namesOut;
+            SafeArray <BSTR> valuesOut;
+            SafeArray <ULONG64> timestampsOut;
+            SafeArray <BSTR> flagsOut;
+            hrc = pConsole->mControl->PullGuestProperties(ComSafeArrayAsOutParam(namesOut),
+                                                ComSafeArrayAsOutParam(valuesOut),
+                                                ComSafeArrayAsOutParam(timestampsOut),
+                                                ComSafeArrayAsOutParam(flagsOut));         H();
+            size_t cProps = namesOut.size();
+            if (   valuesOut.size() != cProps
+                || timestampsOut.size() != cProps
+                || flagsOut.size() != cProps
+               )
+                rc = VERR_INVALID_PARAMETER;
+
+            std::vector <Utf8Str> utf8Names, utf8Values, utf8Flags;
+            std::vector <char *> names, values, flags;
+            std::vector <ULONG64> timestamps;
+            for (unsigned i = 0; i < cProps && RT_SUCCESS(rc); ++i)
+                if (   !VALID_PTR(namesOut[i])
+                    || !VALID_PTR(valuesOut[i])
+                    || !VALID_PTR(flagsOut[i])
+                   )
+                    rc = VERR_INVALID_POINTER;
+            for (unsigned i = 0; i < cProps && RT_SUCCESS(rc); ++i)
+            {
+                utf8Names.push_back(Bstr(namesOut[i]));
+                utf8Values.push_back(Bstr(valuesOut[i]));
+                timestamps.push_back(timestampsOut[i]);
+                utf8Flags.push_back(Bstr(flagsOut[i]));
+                if (   utf8Names.back().isNull()
+                    || utf8Values.back().isNull()
+                    || utf8Flags.back().isNull()
+                   )
+                    throw std::bad_alloc();
+            }
+            for (unsigned i = 0; i < cProps && RT_SUCCESS(rc); ++i)
+            {
+                names.push_back(utf8Names[i].mutableRaw());
+                values.push_back(utf8Values[i].mutableRaw());
+                flags.push_back(utf8Flags[i].mutableRaw());
+            }
+            names.push_back(NULL);
+            values.push_back(NULL);
+            timestamps.push_back(0);
+            flags.push_back(NULL);
 
             /* Setup the service. */
-            VBOXHGCMSVCPARM parms[3];
+            VBOXHGCMSVCPARM parms[4];
 
             parms[0].type = VBOX_HGCM_SVC_PARM_PTR;
-            parms[0].u.pointer.addr = pValues;
-            parms[0].u.pointer.size = sizeof(pValues);  /* We don't actually care. */
+            parms[0].u.pointer.addr = &names.front();
+            parms[0].u.pointer.size = 0;  /* We don't actually care. */
             parms[1].type = VBOX_HGCM_SVC_PARM_PTR;
-            parms[1].u.pointer.addr = pTimestamps;
-            parms[1].u.pointer.size = sizeof(pTimestamps);
+            parms[1].u.pointer.addr = &values.front();
+            parms[1].u.pointer.size = 0;  /* We don't actually care. */
             parms[2].type = VBOX_HGCM_SVC_PARM_PTR;
-            parms[2].u.pointer.addr = pFlags;
-            parms[2].u.pointer.size = sizeof(pFlags);
-
-            pConsole->mVMMDev->hgcmHostCall ("VBoxGuestPropSvc", guestProp::SET_CFGM_NODE, 3, &parms[0]);
+            parms[2].u.pointer.addr = &timestamps.front();
+            parms[2].u.pointer.size = 0;  /* We don't actually care. */
+            parms[3].type = VBOX_HGCM_SVC_PARM_PTR;
+            parms[3].u.pointer.addr = &flags.front();
+            parms[3].u.pointer.size = 0;  /* We don't actually care. */
+
+            pConsole->mVMMDev->hgcmHostCall ("VBoxGuestPropSvc", guestProp::SET_PROPS_HOST, 4, &parms[0]);
 
             /* Register the host notification callback */

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3085 +3085 @@
             mHWData->mGuestProperties.push_back(property);
         }
+        if (   SUCCEEDED (rc)
+            && (   matchAll
+                || RTStrSimplePatternMultiMatch (utf8Patterns.raw(), RTSTR_MAX,
+                                                utf8Name.raw(), RTSTR_MAX, NULL)
+              )
+          )
+            mParent->onGuestPropertyChange (mData->mUuid, aName, aValue, aFlags);
     }
     else
@@ -3100 +3107 @@
                                                  &dummy, &dummy64, &dummy);
     }
-    if (   SUCCEEDED (rc)
-        && (   matchAll
-            || RTStrSimplePatternMultiMatch (utf8Patterns.raw(), RTSTR_MAX,
-                                             utf8Name.raw(), RTSTR_MAX, NULL)
-           )
-       )
-        mParent->onGuestPropertyChange (mData->mUuid, aName, aValue, aFlags);
     return rc;
 #endif /* else !defined (VBOX_WITH_GUEST_PROPS) */

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

@@ -494 +494 @@
     static DECLCALLBACK(int)    doGuestPropNotification (void *pvExtension, uint32_t,
                                                          void *pvParms, uint32_t cbParms);
+    HRESULT doEnumerateGuestProperties (INPTR BSTR aPatterns,
+                                        ComSafeArrayOut(BSTR, aNames),
+                                        ComSafeArrayOut(BSTR, aValues),
+                                        ComSafeArrayOut(ULONG64, aTimestamps),
+                                        ComSafeArrayOut(BSTR, aFlags));
 #endif