Changeset 37256 in vbox

Timestamp:

May 30, 2011 12:39:03 PM (14 years ago)

Author:

vboxsync

Message:

VBoxGuestR3LibAdditions: VersionEx -> VersionExt, refacatored code.

Location:

trunk/src/VBox/Additions/common

Files:

• VBoxGuestLib/VBoxGuestR3LibAdditions.cpp (modified) (8 diffs)
• VBoxService/VBoxServiceVMInfo.cpp (modified) (2 diffs)

trunk/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibAdditions.cpp

@@ -179 +179 @@
 
 
+int vbglR3QueryRegistryString(HKEY hKey, const char *pszValName, char *pszBuffer, size_t cchBuffer)
+{
+    AssertReturn(pszValName, VERR_INVALID_PARAMETER);
+    AssertReturn(pszBuffer, VERR_INVALID_POINTER);
+    AssertReturn(cchBuffer, VERR_INVALID_PARAMETER);
+
+    int rc = VINF_SUCCESS;
+
+    DWORD dwType;
+    DWORD dwSize = (DWORD)cchBuffer;
+    LONG lRet = RegQueryValueEx(hKey, pszValName, NULL, &dwType, (BYTE*)(LPCTSTR)pszBuffer, &dwSize);
+    if (lRet == ERROR_SUCCESS)
+    {
+        rc = dwType == REG_SZ
+                     ? VINF_SUCCESS : VERR_INVALID_PARAMETER;
+    }
+    else
+        rc = RTErrConvertFromWin32(lRet);
+    return rc;
+}
+
+
 /**
  * Retrieves the installed Guest Additions version and/or revision.
@@ -186 +208 @@
  *                      (major.minor.build). NULL is accepted. The returned
  *                      pointer must be freed using RTStrFree().*
- * @param   ppszVerEx   Receives pointer of allocated full version string
+ * @param   ppszVerExt  Receives pointer of allocated full version string
  *                      (raw version + vendor suffix(es)). NULL is
  *                      accepted. The returned pointer must be freed using
@@ -194 +216 @@
  *                      RTStrFree().
  */
-VBGLR3DECL(int) VbglR3GetAdditionsVersion(char **ppszVer, char **ppszVerEx, char **ppszRev)
+VBGLR3DECL(int) VbglR3GetAdditionsVersion(char **ppszVer, char **ppszVerExt, char **ppszRev)
 {
     /*
@@ -201 +223 @@
     if (ppszVer)
         *ppszVer   = NULL;
-    if (ppszVerEx)
-        *ppszVerEx = NULL;
+    if (ppszVerExt)
+        *ppszVerExt = NULL;
     if (ppszRev)
         *ppszRev   = NULL;
@@ -215 +237 @@
          * Version.
          */
-        /** @todo r=bird: This code has broken error handling: 1. Errors are
-         *        overwritten and hidden, leaving return values undefined.
-         *        2. On error, memory could be leaked.
-         *  Also, just use a static 32-byte buffer instead of allocating
-         *  three!  In fact, why not just factor out the string value query? */
-        LONG l;
-        DWORD dwType;
-        DWORD dwSize = 32;
-        char *pszTmp;
+        char szTemp[32];
         if (ppszVer)
         {
-            pszTmp = (char*)RTMemAlloc(dwSize);
-            if (pszTmp)
-            {
-                l = RegQueryValueEx(hKey, "Version", NULL, &dwType, (BYTE*)(LPCTSTR)pszTmp, &dwSize);
-                if (l == ERROR_SUCCESS)
-                {
-                    if (dwType == REG_SZ)
-                        rc = RTStrDupEx(ppszVer, pszTmp);
-                    else
-                        rc = VERR_INVALID_PARAMETER;
-                }
-                else
-                {
-                    rc = RTErrConvertFromWin32(l);
-                }
-                RTMemFree(pszTmp);
-            }
-            else
-                rc = VERR_NO_MEMORY;
-        }
-        if (ppszVerEx)
-        {
-            dwSize = 32; /* Reset */
-            pszTmp = (char*)RTMemAlloc(dwSize);
-            if (pszTmp)
-            {
-                l = RegQueryValueEx(hKey, "VersionEx", NULL, &dwType, (BYTE*)(LPCTSTR)pszTmp, &dwSize);
-                if (l == ERROR_SUCCESS)
-                {
-                    if (dwType == REG_SZ)
-                        rc = RTStrDupEx(ppszVerEx, pszTmp);
-                    else
-                        rc = VERR_INVALID_PARAMETER;
-                }
-                else
-                {
-                    rc = RTErrConvertFromWin32(l);
-                }
-                RTMemFree(pszTmp);
-            }
-            else
-                rc = VERR_NO_MEMORY;
+            rc = vbglR3QueryRegistryString(hKey, "Version", szTemp, sizeof(szTemp));
+            if (RT_SUCCESS(rc))
+                rc = RTStrDupEx(ppszVer, szTemp);
+        }
+
+        if (   RT_SUCCESS(rc)
+            && ppszVerExt)
+        {
+            rc = vbglR3QueryRegistryString(hKey, "VersionExt", szTemp, sizeof(szTemp));
+            if (RT_SUCCESS(rc))
+                rc = RTStrDupEx(ppszVerExt, szTemp);
         }
 
@@ -273 +256 @@
          * Revision.
          */
-        if (ppszRev)
-        {
-            dwSize = 32; /* Reset */
-            pszTmp = (char*)RTMemAlloc(dwSize);
-            if (pszTmp)
-            {
-                l = RegQueryValueEx(hKey, "Revision", NULL, &dwType, (BYTE*)(LPCTSTR)pszTmp, &dwSize);
-                if (l == ERROR_SUCCESS)
-                {
-                    if (dwType == REG_SZ)
-                        rc = RTStrDupEx(ppszRev, pszTmp);
-                    else
-                        rc = VERR_INVALID_PARAMETER;
-                }
-                else
-                {
-                    rc = RTErrConvertFromWin32(l);
-                }
-                RTMemFree(pszTmp);
-            }
-            else
-                rc = VERR_NO_MEMORY;
-
-            if (RT_FAILURE(rc) && ppszVer)
-            {
+        if (   RT_SUCCESS(rc)
+            && ppszRev)
+        {
+            rc = vbglR3QueryRegistryString(hKey, "Revision", szTemp, sizeof(szTemp));
+            if (RT_SUCCESS(rc))
+                rc = RTStrDupEx(ppszRev, szTemp);
+        }
+
+        int rc2 = vbglR3CloseAdditionsWinStoragePath(hKey);
+        if (RT_SUCCESS(rc))
+            rc = rc2;
+        else
+        {
+            /* Clean up allocated strings on error. */
+            if (*ppszVer)
                 RTStrFree(*ppszVer);
-                *ppszVer = NULL;
-            }
-        }
-        rc = vbglR3CloseAdditionsWinStoragePath(hKey);
+            if (*ppszVerExt)
+                RTStrFree(*ppszVerExt);
+            if (*ppszRev)
+                RTStrFree(*ppszRev);
+        }
     }
     else
@@ -310 +284 @@
          * into this binary.
          */
-        rc = vbglR3GetAdditionsCompileTimeVersion(ppszVer, ppszVerEx, ppszRev);
+        rc = vbglR3GetAdditionsCompileTimeVersion(ppszVer, ppszVerExt, ppszRev);
     }
     return rc;
@@ -318 +292 @@
      * On non-Windows platforms just return the compile-time version string.
      */
-    return vbglR3GetAdditionsCompileTimeVersion(ppszVer, ppszVerEx, ppszRev);
+    return vbglR3GetAdditionsCompileTimeVersion(ppszVer, ppszVerExt, ppszRev);
 #endif /* !RT_OS_WINDOWS */
 }

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

@@ -190 +190 @@
      */
     char *pszAddVer;
-    char *pszAddVerEx;
+    char *pszAddVerExt;
     char *pszAddRev;
-    rc = VbglR3GetAdditionsVersion(&pszAddVer, &pszAddVerEx, &pszAddRev);
+    rc = VbglR3GetAdditionsVersion(&pszAddVer, &pszAddVerExt, &pszAddRev);
     VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/Version",
                           "%s", RT_FAILURE(rc) ? "" : pszAddVer);
-    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/VersionEx",
-                          "%s", RT_FAILURE(rc) ? "" : pszAddVerEx);
+    VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/VersionExt",
+                          "%s", RT_FAILURE(rc) ? "" : pszAddVerExt);
     VBoxServiceWritePropF(g_uVMInfoGuestPropSvcClientID, "/VirtualBox/GuestAdd/Revision",
                           "%s", RT_FAILURE(rc) ? "" : pszAddRev);
@@ -202 +202 @@
     {
         RTStrFree(pszAddVer);
-        RTStrFree(pszAddVerEx);
+        RTStrFree(pszAddVerExt);
         RTStrFree(pszAddRev);
     }