Changeset 25354 in vbox for trunk/src

Timestamp:

Dec 14, 2009 10:15:38 AM (15 years ago)

Author:

vboxsync

Message:

GuestProperties/service.cpp: only return VINF_PERMISSION_DENIED when the requesting side has been globally made read-only.

File:

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

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

@@ -226 +226 @@
     /**
      * Check whether we have permission to change a property.
-     * @returns VINF_SUCCESS if we do, VERR_PERMISSION_DENIED otherwise
+     *
+     * @returns Strict VBox status code.
+     * @retval  VINF_SUCCESS if we do.
+     * @retval  VERR_PERMISSION_DENIED if the value is read-only for the requesting
+     *          side.
+     * @retval  VINF_PERMISSION_DENIED if the side is globally marked read-only.
+     *
      * @param   eFlags   the flags on the property in question
      * @param   isGuest  is the guest or the host trying to make the change?
@@ -232 +238 @@
     int checkPermission(ePropFlags eFlags, bool isGuest)
     {
-        if (   (isGuest && (eFlags & RDONLYGUEST))
-            || (isGuest && (meGlobalFlags & RDONLYGUEST))
-            || (!isGuest && (eFlags & RDONLYHOST))
-           )
+        if (eFlags & (isGuest ? RDONLYGUEST : RDONLYHOST))
+            return VERR_PERMISSION_DENIED;
+        if (isGuest && (meGlobalFlags & RDONLYGUEST))
             return VINF_PERMISSION_DENIED;
-
         return VINF_SUCCESS;
     }
@@ -649 +653 @@
     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))
+    if (RT_SUCCESS(rc))
+    {
+        /*
+         * If the property already exists, check its flags to see if we are allowed
+         * to change it.
+         */
+        PropertyList::iterator it;
+        bool found = false;
         for (it = mProperties.begin(); it != mProperties.end(); ++it)
             if (it->mName.compare(pcszName) == 0)
@@ -663 +667 @@
                 break;
             }
-    if (RT_SUCCESS(rc))
+
         rc = checkPermission(found ? (ePropFlags)it->mFlags : NILFLAG,
                              isGuest);
-
-    if (rc == VINF_PERMISSION_DENIED)
-        return rc;
-
-    /*
-     * Set the actual value
-     */
-    if (RT_SUCCESS(rc))
-    {
-        if (found)
-        {
-            it->mValue = pcszValue;
-            it->mTimestamp = u64TimeNano;
-            it->mFlags = fFlags;
+        if (rc == VINF_SUCCESS)
+        {
+            /*
+             * Set the actual value
+             */
+            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 (isGuest)  /* Notify the host even for properties that the host
+            //                * changed.  Less efficient, but ensures consistency. */
+                doNotifications(pcszName, u64TimeNano);
+            Log2(("Set string %s, rc=%Rrc, value=%s\n", pcszName, rc, pcszValue));
         }
-        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. */
-            doNotifications(pcszName, u64TimeNano);
-        Log2(("Set string %s, rc=%Rrc, value=%s\n", pcszName, rc, pcszValue));
-    }
+    }
+
     LogFlowThisFunc(("rc = %Rrc\n", rc));
     return rc;
@@ -727 +726 @@
     else
         rc = VERR_INVALID_PARAMETER;
-
-    /*
-     * 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))
+    if (RT_SUCCESS(rc))
+    {
+        /*
+         * If the property exists, check its flags to see if we are allowed
+         * to change it.
+         */
+        PropertyList::iterator it;
+        bool found = false;
         for (it = mProperties.begin(); it != mProperties.end(); ++it)
             if (it->mName.compare(pcszName) == 0)
@@ -741 +740 @@
                 break;
             }
-    if (RT_SUCCESS(rc))
-        rc = checkPermission(found ? (ePropFlags)it->mFlags :
-                             NILFLAG, isGuest);
-
-    if (rc == VINF_PERMISSION_DENIED)
-        return rc;
-
-    /*
-     * And delete the property if all is well.
-     */
-    if (RT_SUCCESS(rc) && found)
-    {
-        RTTIMESPEC time;
-        uint64_t u64Timestamp = RTTimeSpecGetNano(RTTimeNow(&time));
-        mProperties.erase(it);
-        // if (isGuest)  /* Notify the host even for properties that the host
-        //                * changed.  Less efficient, but ensures consistency. */
-            doNotifications(pcszName, u64Timestamp);
-    }
+        rc = checkPermission(found ? (ePropFlags)it->mFlags : NILFLAG,
+                             isGuest);
+
+        /*
+         * And delete the property if all is well.
+         */
+        if (rc == VINF_SUCCESS && found)
+        {
+            RTTIMESPEC time;
+            uint64_t u64Timestamp = RTTimeSpecGetNano(RTTimeNow(&time));
+            mProperties.erase(it);
+            // if (isGuest)  /* Notify the host even for properties that the host
+            //                * changed.  Less efficient, but ensures consistency. */
+                doNotifications(pcszName, u64Timestamp);
+        }
+    }
+
     LogFlowThisFunc(("rc = %Rrc\n", rc));
     return rc;
@@ -1069 +1066 @@
                 if (prop.Matches(pszPatterns))
                 {
-                    GuestCall call = *it;
-                    int rc2 = getNotificationWriteOut(call.mParms, prop);
+                    GuestCall curCall = *it;
+                    int rc2 = getNotificationWriteOut(curCall.mParms, prop);
                     if (RT_SUCCESS(rc2))
-                        rc2 = call.mRc;
-                    mpHelpers->pfnCallComplete (call.mHandle, rc2);
+                        rc2 = curCall.mRc;
+                    mpHelpers->pfnCallComplete(curCall.mHandle, rc2);
                     it = mGuestWaiters.erase(it);
                 }