Changeset 29026 in vbox for trunk/src/VBox/Additions/common/VBoxService

Timestamp:

May 4, 2010 2:18:27 PM (15 years ago)

Author:

vboxsync

Message:

VBoxServiceVMInfo/PropCache: r=bird: todos in the code. Fixed two odd semaphore exit bugs.

Location:

trunk/src/VBox/Additions/common/VBoxService

Files:

• Makefile.kmk (modified) (2 diffs)
• VBoxServiceInternal.h (modified) (5 diffs)
• VBoxServicePropCache.cpp (modified) (16 diffs)
• VBoxServicePropCache.h (modified) (1 diff)
• VBoxServiceVMInfo.cpp (modified) (18 diffs)

trunk/src/VBox/Additions/common/VBoxService/Makefile.kmk

@@ -55 +55 @@
         VBoxServiceTimeSync.cpp \
         VBoxServiceUtils.cpp \
-        VBoxServiceStats.cpp \
-        VBoxServicePropCache.cpp
+        VBoxServiceStats.cpp
 ifdef VBOX_WITH_GUEST_CONTROL
  VBoxService_SOURCES    += \
-        VBoxServiceControl.cpp \
-        VBoxServiceControlExec.cpp
+        VBoxServiceControl.cpp \
+        VBoxServiceControlExec.cpp
 endif
 ifdef VBOX_WITH_MEMBALLOON
  VBoxService_SOURCES    += \
-        VBoxServiceBalloon.cpp
+        VBoxServiceBalloon.cpp
 endif
 ifdef VBOX_WITH_PAGE_SHARING
  VBoxService_SOURCES    += \
-        VBoxServicePageSharing.cpp
+        VBoxServicePageSharing.cpp
 endif
 ifdef VBOX_WITH_GUEST_PROPS
@@ -74 +73 @@
         VBoxServiceVMInfo-win.cpp
  VBoxService_SOURCES     += \
-        VBoxServiceVMInfo.cpp
+        VBoxServiceVMInfo.cpp \
+        VBoxServicePropCache.cpp
  if1of ($(KBUILD_TARGET), win)
   VBoxService_SOURCES    += \

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceInternal.h

@@ -93 +93 @@
 
 #ifdef RT_OS_WINDOWS
+
 /** The service name (needed for mutex creation on Windows). */
 # define VBOXSERVICE_NAME           "VBoxService"
@@ -122 +123 @@
 /** Function prototypes for dynamic loading. */
 typedef DWORD (WINAPI *PFNWTSGETACTIVECONSOLESESSIONID)(void);
+
 #endif /* RT_OS_WINDOWS */
-
 #ifdef VBOX_WITH_GUEST_CONTROL
+
 enum VBOXSERVICECTRLTHREADDATATYPE
 {
@@ -194 +196 @@
  * For buffering process input supplied by the client.
  */
-typedef struct
+typedef struct VBOXSERVICECTRLSTDINBUF
 {
     /** The mount of buffered data. */
@@ -211 +213 @@
 /** Pointer to a standard input buffer. */
 typedef VBOXSERVICECTRLSTDINBUF *PVBOXSERVICECTRLSTDINBUF;
+
 #endif /* VBOX_WITH_GUEST_CONTROL */
-
 #ifdef VBOX_WITH_GUEST_PROPS
+
 /**
  * A guest property cache.
  */
-typedef struct
-{    
+typedef struct VBOXSERVICEVEPROPCACHE
+{
     /** The client ID for HGCM communication. */
-    uint32_t   uClientID;
-    RTLISTNODE Node;
-    RTSEMMUTEX Mutex;
+    uint32_t    uClientID;
+    /** List of VBOXSERVICEVEPROPCACHEENTRY nodes.
+     * @todo r=bird: Node -> ListSomething, please.  "Node" is very nondescript and
+     *       makes the list-head vs. list-node confusion greater. */
+    RTLISTNODE  Node;
+    /** @todo Use a RTCRITSECT. RTSEMMUTEXes are deprecated in ring-3. */
+    RTSEMMUTEX  Mutex;
 } VBOXSERVICEVEPROPCACHE;
 /** Pointer to a guest property cache. */
@@ -228 +235 @@
 
 /**
- * Handling guest properties used by the VM information service.
- */
-typedef struct
-{    
+ * An entry in the property cache (VBOXSERVICEVEPROPCACHE).
+ */
+typedef struct VBOXSERVICEVEPROPCACHEENTRY
+{
     /** Node. */
-    RTLISTNODE Node;
+    RTLISTNODE  Node;
     /** Name (and full path) of guest property. */
-    char    *pszName;
+    char       *pszName;
     /** The last value stored (for reference). */
-    char    *pszValue;
-    /** Reset value to write if property is temporary. */
-    char    *pszValueReset;
+    char       *pszValue;
+    /** Reset value to write if property is temporary.  If NULL, it will be
+     *  deleted. */
+    char       *pszValueReset;
     /** Flags. */
-    uint32_t uFlags;
+    uint32_t    fFlags;
 } VBOXSERVICEVEPROPCACHEENTRY;
 /** Pointer to a cached guest property. */
 typedef VBOXSERVICEVEPROPCACHEENTRY *PVBOXSERVICEVEPROPCACHEENTRY;
+
 #endif /* VBOX_WITH_GUEST_PROPS */
 

trunk/src/VBox/Additions/common/VBoxService/VBoxServicePropCache.cpp

@@ -30 +30 @@
 #include "VBoxServicePropCache.h"
 
-#ifdef VBOX_WITH_GUEST_PROPS
-
-/** @todo Docs */
-int VBoxServicePropCacheInit(PVBOXSERVICEVEPROPCACHE pCache, uint32_t u32ClientId)
-{
-    AssertPtr(pCache);
-    /** @todo Prevent init the cache twice! */
+
+/**
+ * Initializes the property cache.
+ *
+ * @returns IPRT status code.
+ * @param   pCache          Pointer to the cache.
+ * @param   uClientId       The HGCM handle of to the guest property service.
+ */
+int VBoxServicePropCacheInit(PVBOXSERVICEVEPROPCACHE pCache, uint32_t uClientId)
+{
+    AssertPtr(pCache);
+    /** @todo Prevent init the cache twice!
+     *  r=bird: Use a magic, or/and abstract the whole cache by rename this function
+     *  VBoxServicePropCacheCreate(). */
     RTListInit(&pCache->Node);
-    pCache->uClientID = u32ClientId;
+    pCache->uClientID = uClientId;
     return RTSemMutexCreate(&pCache->Mutex);
 }
 
 
-/** @todo Docs */
+/** @todo Docs
+ * @todo this looks internal to me, nobody should need to access the
+ *       structures directly here. */
 PVBOXSERVICEVEPROPCACHEENTRY VBoxServicePropCacheFind(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t uFlags)
 {
     AssertPtr(pCache);
     AssertPtr(pszName);
-    /* This is a O(n) lookup, maybe improve this later to O(1) using a map. */
+    /** @todo This is a O(n) lookup, maybe improve this later to O(1) using a
+     *        map.
+     *  r=bird: Use a string space (RTstrSpace*). That is O(log n) in its current
+     *        implementation (AVL tree). However, this is not important at the
+     *        moment. */
     PVBOXSERVICEVEPROPCACHEENTRY pNodeIt, pNode = NULL;
     if (RT_SUCCESS(RTSemMutexRequest(pCache->Mutex, RT_INDEFINITE_WAIT)))
-    {    
+    {
         RTListForEach(&pCache->Node, pNodeIt, VBOXSERVICEVEPROPCACHEENTRY, Node)
         {
@@ -67 +80 @@
 
 /** @todo Docs */
-int VBoxServicePropCacheUpdateEntry(PVBOXSERVICEVEPROPCACHE pCache, 
-                                    const char *pszName, uint32_t u32Flags, const char *pszValueReset)
+int VBoxServicePropCacheUpdateEntry(PVBOXSERVICEVEPROPCACHE pCache,
+                                    const char *pszName, uint32_t fFlags, const char *pszValueReset)
 {
     AssertPtr(pCache);
@@ -79 +92 @@
         if (RT_SUCCESS(rc))
         {
-            pNode->uFlags = u32Flags;
+            pNode->fFlags = fFlags;
             if (pszValueReset)
             {
@@ -100 +113 @@
  * @returns VBox status code. Errors will be logged.
  *
- * @param   u32ClientId     The HGCM client ID for the guest property session.
+ * @param   pCache          The property cache.
  * @param   pszName         The property name.
  * @param   pszValueFormat  The property format string.  If this is NULL then
@@ -129 +142 @@
  * @returns VBox status code. Errors will be logged.
  *
- * @param   u32ClientId     The HGCM client ID for the guest property session.
+ * @param   pCache          The property cache.
  * @param   pszName         The property name.
  * @param   pszValueFormat  The property format string.  If this is NULL then
@@ -135 +148 @@
  * @param   ...             Format arguments.
  */
-int VBoxServicePropCacheUpdateEx(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t u32Flags, const char *pszValueReset, const char *pszValueFormat, ...)
+int VBoxServicePropCacheUpdateEx(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t fFlags,
+                                 const char *pszValueReset, const char *pszValueFormat, ...)
 {
     AssertPtr(pCache);
@@ -141 +155 @@
     AssertPtr(pszName);
 
+    /*
+     * Format the value first.
+     */
     char *pszValue = NULL;
     if (pszValueFormat)
@@ -148 +165 @@
         RTStrAPrintfV(&pszValue, pszValueFormat, va);
         va_end(va);
+        if (!pszValue)
+            return VERR_NO_STR_MEMORY;
     }
 
@@ -156 +175 @@
     if (RT_SUCCESS(rc))
     {
-    
+
         if (pNode == NULL)
-        {          
+        {
             pNode = (PVBOXSERVICEVEPROPCACHEENTRY)RTMemAlloc(sizeof(VBOXSERVICEVEPROPCACHEENTRY));
-            AssertPtrReturn(pNode, VERR_NO_MEMORY);
-    
+            if (RT_UNLIKELY(!pNode))
+            {
+                RTSemMutexRelease(pCache->Mutex);
+                return VERR_NO_MEMORY;
+            }
+
             pNode->pszName = RTStrDup(pszName);
-            pNode->pszValue = NULL; 
-            pNode->uFlags = 0;
+            pNode->pszValue = NULL;
+            pNode->fFlags = 0;
             pNode->pszValueReset = NULL;
-    
+
             /*rc =*/ RTListAppend(&pCache->Node, &pNode->Node);
         }
-    
+
         AssertPtr(pNode);
         if (pszValue) /* Do we have a value to check for? */
@@ -175 +198 @@
             bool fUpdate = false;
             /* Always update this property, no matter what? */
-            if (pNode->uFlags & VBOXSERVICEPROPCACHEFLAG_ALWAYS_UPDATE)
+            if (pNode->fFlags & VBOXSERVICEPROPCACHEFLAG_ALWAYS_UPDATE)
                 fUpdate = true;
             /* Did the value change so we have to update? */
@@ -181 +204 @@
                 fUpdate = true;
             /* No value stored at the moment but we have a value now? */
-            else if (pNode->pszValue == NULL) 
+            else if (pNode->pszValue == NULL)
                 fUpdate = true;
-    
+
             if (fUpdate)
             {
                 /* Write the update. */
                 rc = VBoxServiceWritePropF(pCache->uClientID, pNode->pszName, pszValue);
-                if (pNode->pszValue)
-                    RTStrFree(pNode->pszValue);
+                RTStrFree(pNode->pszValue);
                 pNode->pszValue = RTStrDup(pszValue);
             }
             else
+                /** @todo r=bird: Add a VINF_NO_CHANGE status code to iprt/err.h and use it. */
                 rc = VINF_ALREADY_INITIALIZED; /* No update needed. */
         }
-        else 
+        else
         {
             /* No value specified. Deletion (or no action required). */
@@ -201 +224 @@
             {
                 /* Delete property (but do not remove from cache) if not deleted yet. */
-                rc = VBoxServiceWritePropF(pCache->uClientID, pNode->pszName, NULL);
                 RTStrFree(pNode->pszValue);
                 pNode->pszValue = NULL;
+                rc = VBoxServiceWritePropF(pCache->uClientID, pNode->pszName, NULL);
             }
             else
+                /** @todo r=bird: Use VINF_NO_CHANGE. */
                 rc = VINF_ALREADY_INITIALIZED; /* No update needed. */
         }
-    
+
         /* Update rest of the fields. */
         if (pszValueReset)
@@ -216 +240 @@
             pNode->pszValueReset = RTStrDup(pszValueReset);
         }
-        if (u32Flags)
-            pNode->uFlags = u32Flags;
-     
-        /* Delete temp stuff. */
-        if (pszValue)
-            RTStrFree(pszValue);   
-    
+        if (fFlags)
+            pNode->fFlags = fFlags;
+
         /* Release cache. */
         int rc2 = RTSemMutexRelease(pCache->Mutex);
@@ -228 +248 @@
             rc2 = rc;
     }
+
+    /* Delete temp stuff. */
+    RTStrFree(pszValue);
+
     return rc;
 }
 
 
-/** @todo Docs */
+/**
+ * Reset all temporary properties and destroy the cache.
+ *
+ * @param   pCache          The property cache.
+ */
 void VBoxServicePropCacheDestroy(PVBOXSERVICEVEPROPCACHE pCache)
 {
@@ -240 +268 @@
     RTListForEach(&pCache->Node, pNode, VBOXSERVICEVEPROPCACHEENTRY, Node)
     {
-        if ((pNode->uFlags & VBOXSERVICEPROPCACHEFLAG_TEMPORARY) == 0)
-        {
-            if (pNode->pszValueReset) /* Write reset value? */
-            {
-                /* rc = */VBoxServiceWritePropF(pCache->uClientID, pNode->pszName, pNode->pszValueReset);
-            }
-            else /* Delete value. */
-                /* rc = */VBoxServiceWritePropF(pCache->uClientID, pNode->pszName, NULL);
-        }
+        if ((pNode->fFlags & VBOXSERVICEPROPCACHEFLAG_TEMPORARY) == 0)
+            VBoxServiceWritePropF(pCache->uClientID, pNode->pszName, pNode->pszValueReset);
 
         AssertPtr(pNode->pszName);
         RTStrFree(pNode->pszName);
-        if (pNode->pszValue)
-            RTStrFree(pNode->pszValue);
-        if (pNode->pszValueReset)
-            RTStrFree(pNode->pszValueReset);
-        pNode->uFlags = 0;
+        RTStrFree(pNode->pszValue);
+        RTStrFree(pNode->pszValueReset);
+        pNode->fFlags = 0;
     }
 
@@ -264 +283 @@
         PVBOXSERVICEVEPROPCACHEENTRY pNext = RTListNodeGetNext(&pNode->Node, VBOXSERVICEVEPROPCACHEENTRY, Node);
         RTListNodeRemove(&pNode->Node);
+        /** @todo r=bird: hrm. missing RTMemFree(pNode)? Why don't you just combine the
+         *        two loops? */
+
         if (pNext && RTListNodeIsLast(&pCache->Node, &pNext->Node))
             break;
         pNode = pNext;
     }
+
     /* Destroy mutex. */
     RTSemMutexDestroy(pCache->Mutex);
 }
-#endif /* VBOX_WITH_GUEST_PROPS */
+

trunk/src/VBox/Additions/common/VBoxService/VBoxServicePropCache.h

@@ -26 +26 @@
 #define VBOXSERVICEPROPCACHEFLAG_ALWAYS_UPDATE      RT_BIT(2)
 
-int VBoxServicePropCacheInit(PVBOXSERVICEVEPROPCACHE pCache, uint32_t u32ClientId);
+int VBoxServicePropCacheInit(PVBOXSERVICEVEPROPCACHE pCache, uint32_t uClientId);
 PVBOXSERVICEVEPROPCACHEENTRY VBoxServicePropCacheFind(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t uFlags);
-int VBoxServicePropCacheUpdateEntry(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t uFlags, const char *pszValueReset);
+int VBoxServicePropCacheUpdateEntry(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t fFlags, const char *pszValueReset);
 int VBoxServicePropCacheUpdate(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, const char *pszValueFormat, ...);
-int VBoxServicePropCacheUpdateEx(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t u32Flags, const char *pszValueReset, const char *pszValueFormat, ...);
+int VBoxServicePropCacheUpdateEx(PVBOXSERVICEVEPROPCACHE pCache, const char *pszName, uint32_t fFlags, const char *pszValueReset, const char *pszValueFormat, ...);
 void VBoxServicePropCacheDestroy(PVBOXSERVICEVEPROPCACHE pCache);
 #endif

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceVMInfo.cpp

@@ -61 +61 @@
 *******************************************************************************/
 /** The vminfo interval (millseconds). */
-uint32_t                        g_VMInfoInterval = 0;
+static uint32_t                 g_cMsVMInfoInterval = 0;
 /** The semaphore we're blocking on. */
-static RTSEMEVENTMULTI          g_VMInfoEvent = NIL_RTSEMEVENTMULTI;
+static RTSEMEVENTMULTI          g_hVMInfoEvent = NIL_RTSEMEVENTMULTI;
 /** The guest property service client ID. */
-static uint32_t                 g_VMInfoGuestPropSvcClientID = 0;
+static uint32_t                 g_uVMInfoGuestPropSvcClientID = 0;
 /** Number of logged in users in OS. */
 static uint32_t                 g_cVMInfoLoggedInUsers = UINT32_MAX;
@@ -87 +87 @@
     else if (!strcmp(argv[*pi], "--vminfo-interval"))
         rc = VBoxServiceArgUInt32(argc, argv, "", pi,
-                                  &g_VMInfoInterval, 1, UINT32_MAX - 1);
+                                  &g_cMsVMInfoInterval, 1, UINT32_MAX - 1);
     return rc;
 }
@@ -99 +99 @@
      * Then create the event sem to block on.
      */
-    if (!g_VMInfoInterval)
-        g_VMInfoInterval = g_DefaultInterval * 1000;
-    if (!g_VMInfoInterval)
-        g_VMInfoInterval = 10 * 1000;
-
-    int rc = RTSemEventMultiCreate(&g_VMInfoEvent);
+    if (!g_cMsVMInfoInterval)
+        g_cMsVMInfoInterval = g_DefaultInterval * 1000;
+    if (!g_cMsVMInfoInterval)
+        g_cMsVMInfoInterval = 10 * 1000;
+
+    int rc = RTSemEventMultiCreate(&g_hVMInfoEvent);
     AssertRCReturn(rc, rc);
 
@@ -117 +117 @@
 #endif
 
-    rc = VbglR3GuestPropConnect(&g_VMInfoGuestPropSvcClientID);
+    rc = VbglR3GuestPropConnect(&g_uVMInfoGuestPropSvcClientID);
     if (RT_SUCCESS(rc))
-        VBoxServiceVerbose(3, "Property Service Client ID: %#x\n", g_VMInfoGuestPropSvcClientID);
+        VBoxServiceVerbose(3, "Property Service Client ID: %#x\n", g_uVMInfoGuestPropSvcClientID);
     else
     {
         VBoxServiceError("Failed to connect to the guest property service! Error: %Rrc\n", rc);
-        RTSemEventMultiDestroy(g_VMInfoEvent);
-        g_VMInfoEvent = NIL_RTSEMEVENTMULTI;
+        RTSemEventMultiDestroy(g_hVMInfoEvent);
+        g_hVMInfoEvent = NIL_RTSEMEVENTMULTI;
     }
 
     if (RT_SUCCESS(rc))
     {
-        VBoxServicePropCacheInit(&g_VMInfoPropCache, g_VMInfoGuestPropSvcClientID);
+        VBoxServicePropCacheInit(&g_VMInfoPropCache, g_uVMInfoGuestPropSvcClientID);
+
+        /** @todo r=bird: Setting Net/Count to 0 here is wrong and will confuse users.
+         *        Besides, because it is the beacon updating it implies that all the
+         *        other values are up to date too, but they aren't
+         *        (VBoxServiceVMInfoWriteFixedProperties hasn't been called yet for
+         *        instance).  I would suggest changing these statements to
+         *        "declarations" or moving them. */
 
         /*
@@ -135 +142 @@
          * Passing NULL as an actual value does not write the properties yet.
          */
-        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/OS/LoggedInUsersList", 
+        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/OS/LoggedInUsersList",
                                      VBOXSERVICEPROPCACHEFLAG_TEMPORARY, NULL /* Delete on exit */, NULL);
-        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/OS/LoggedInUsers", 
+        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/OS/LoggedInUsers",
                                      VBOXSERVICEPROPCACHEFLAG_TEMPORARY, "0", NULL);
-        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/OS/NoLoggedInUsers", 
+        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/OS/NoLoggedInUsers",
                                      VBOXSERVICEPROPCACHEFLAG_TEMPORARY, "true", NULL);
         /*
@@ -146 +153 @@
          * properties are no longer valid.
          */
-        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/Net/Count", 
+        VBoxServicePropCacheUpdateEx(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/Net/Count",
                                      VBOXSERVICEPROPCACHEFLAG_TEMPORARY | VBOXSERVICEPROPCACHEFLAG_ALWAYS_UPDATE, "0", NULL);
     }
@@ -165 +172 @@
     char szInfo[256];
     int rc = RTSystemQueryOSInfo(RTSYSOSINFO_PRODUCT, szInfo, sizeof(szInfo));
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/Product",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/Product",
                           "%s", RT_FAILURE(rc) ? "" : szInfo);
 
     rc = RTSystemQueryOSInfo(RTSYSOSINFO_RELEASE, szInfo, sizeof(szInfo));
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/Release",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/Release",
                           "%s", RT_FAILURE(rc) ? "" : szInfo);
 
     rc = RTSystemQueryOSInfo(RTSYSOSINFO_VERSION, szInfo, sizeof(szInfo));
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/Version",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/Version",
                           "%s", RT_FAILURE(rc) ? "" : szInfo);
 
     rc = RTSystemQueryOSInfo(RTSYSOSINFO_SERVICE_PACK, szInfo, sizeof(szInfo));
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/ServicePack",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestInfo/OS/ServicePack",
                           "%s", RT_FAILURE(rc) ? "" : szInfo);
 
@@ -186 +193 @@
     char *pszAddRev;
     rc = VbglR3GetAdditionsVersion(&pszAddVer, &pszAddRev);
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/Version",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/Version",
                           "%s", RT_FAILURE(rc) ? "" : pszAddVer);
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/Revision",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/Revision",
                           "%s", RT_FAILURE(rc) ? "" : pszAddRev);
     if (RT_SUCCESS(rc))
@@ -202 +209 @@
     char *pszInstDir;
     rc = VbglR3GetAdditionsInstallationPath(&pszInstDir);
-    VBoxServiceWritePropF(g_VMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/InstallDir",
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/InstallDir",
                           "%s", RT_FAILURE(rc) ? "" :  pszInstDir);
     if (RT_SUCCESS(rc))
         RTStrFree(pszInstDir);
 
-    VBoxServiceWinGetComponentVersions(g_VMInfoGuestPropSvcClientID);
+    VBoxServiceWinGetComponentVersions(g_uVMInfoGuestPropSvcClientID);
 #endif
 }
@@ -338 +345 @@
             || g_cVMInfoLoggedInUsers == UINT32_MAX)
         {
-            /* 
+            /*
              * Update this property ONLY if there is a real change from no users to
              * users or vice versa. The only exception is that the initialization
              * forces an update, but only once. This ensures consistent property
-             * settings even if the VM aborted previously. 
+             * settings even if the VM aborted previously.
              */
             if (cUsersInList == 0)
@@ -355 +362 @@
          */
         /** @todo Throw this code into a separate function/module? */
-       int nNumInterfaces = 0;
+       int cInterfaces = 0;
 #ifdef RT_OS_WINDOWS
         SOCKET sd = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
@@ -379 +386 @@
             return -1;
         }
-        nNumInterfaces = nBytesReturned / sizeof(INTERFACE_INFO);
+        cInterfaces = nBytesReturned / sizeof(INTERFACE_INFO);
 #else
         int sd = socket(AF_INET, SOCK_DGRAM, 0);
@@ -400 +407 @@
         ifreq* ifrequest = ifcfg.ifc_req;
         ifreq *ifreqitem = NULL;
-        nNumInterfaces = ifcfg.ifc_len / sizeof(ifreq);
+        cInterfaces = ifcfg.ifc_len / sizeof(ifreq);
 #endif
         char szPropPath [FILENAME_MAX];
         int iCurIface = 0;
 
+        /** @todo r=bird: As mentioned in the defect, this must be written after ALL
+         *        the other values since it indicates that they are up-to-date by its
+         *        timestamp. */
         VBoxServicePropCacheUpdate(&g_VMInfoPropCache, "/VirtualBox/GuestInfo/Net/Count", "%d",
-                                   nNumInterfaces > 1 ? nNumInterfaces-1 : 0);
+                                   cInterfaces > 1 ? cInterfaces-1 : 0);
 
         /** @todo Use GetAdaptersInfo() and GetAdapterAddresses (IPv4 + IPv6) for more information. */
-        for (int i = 0; i < nNumInterfaces; ++i)
+        for (int i = 0; i < cInterfaces; ++i)
         {
             sockaddr_in *pAddress;
@@ -475 +485 @@
             close(sd);
 #endif
+        /** @todo r=bird: if cInterfaces decreased compared to the previous run, zap
+         *        the stale data. */
+
 
         /*
@@ -484 +497 @@
         if (*pfShutdown)
             break;
-        int rc2 = RTSemEventMultiWait(g_VMInfoEvent, g_VMInfoInterval);
+        int rc2 = RTSemEventMultiWait(g_hVMInfoEvent, g_cMsVMInfoInterval);
         if (*pfShutdown)
             break;
@@ -506 +519 @@
 static DECLCALLBACK(void) VBoxServiceVMInfoStop(void)
 {
-    RTSemEventMultiSignal(g_VMInfoEvent);
+    RTSemEventMultiSignal(g_hVMInfoEvent);
 }
 
@@ -515 +528 @@
     int rc;
 
-    if (g_VMInfoEvent != NIL_RTSEMEVENTMULTI)
+    if (g_hVMInfoEvent != NIL_RTSEMEVENTMULTI)
     {
+        /** @todo temporary solution: Zap all values which are not valid
+         *        anymore when VM goes down (reboot/shutdown ). Needs to
+         *        be replaced with "temporary properties" later.
+         *
+         *        One idea is to introduce a (HGCM-)session guest property
+         *        flag meaning that a guest property is only valid as long
+         *        as the HGCM session isn't closed (e.g. guest application
+         *        terminates). [don't remove till implemented]
+         */
+        /** @todo r=bird: Drop the VbglR3GuestPropDelSet call here and make the cache
+         *        remember what we've written. */
         /* Delete the "../Net" branch. */
         const char *apszPat[1] = { "/VirtualBox/GuestInfo/Net/*" };
-        rc = VbglR3GuestPropDelSet(g_VMInfoGuestPropSvcClientID, &apszPat[0], RT_ELEMENTS(apszPat));
+        rc = VbglR3GuestPropDelSet(g_uVMInfoGuestPropSvcClientID, &apszPat[0], RT_ELEMENTS(apszPat));
 
         /* Destroy property cache. */
@@ -525 +549 @@
 
         /* Disconnect from guest properties service. */
-        rc = VbglR3GuestPropDisconnect(g_VMInfoGuestPropSvcClientID);
+        rc = VbglR3GuestPropDisconnect(g_uVMInfoGuestPropSvcClientID);
         if (RT_FAILURE(rc))
             VBoxServiceError("Failed to disconnect from guest property service! Error: %Rrc\n", rc);
-        g_VMInfoGuestPropSvcClientID = 0;
-
-        RTSemEventMultiDestroy(g_VMInfoEvent);
-        g_VMInfoEvent = NIL_RTSEMEVENTMULTI;
+        g_uVMInfoGuestPropSvcClientID = 0;
+
+        RTSemEventMultiDestroy(g_hVMInfoEvent);
+        g_hVMInfoEvent = NIL_RTSEMEVENTMULTI;
     }
 }