Changeset 37141 in vbox for trunk/src/VBox/Additions

Timestamp:

May 18, 2011 3:14:59 PM (14 years ago)

Author:

vboxsync

Message:

VBoxGuestR3LibAdditions.cpp: r=bird: Added todo about leaks and misbeavior of VbglR3GetAdditionsVersion on windows, fixed it in the x-platform code.

File:

• trunk/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibAdditions.cpp (modified) (7 diffs)

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

@@ -37 +37 @@
 
 /**
- * Fallback for vbglR3GetAdditionsVersion.
+ * Fallback for VbglR3GetAdditionsVersion.
  */
 static int vbglR3GetAdditionsCompileTimeVersion(char **ppszVer, char **ppszVerEx, char **ppszRev)
 {
-    /* Raw version string: major.minor.build. */
+    int rc = VINF_SUCCESS;
     if (ppszVer)
-    {
-        *ppszVer = RTStrDup(VBOX_VERSION_STRING_RAW);
-        if (!*ppszVer)
-            return VERR_NO_STR_MEMORY;
-    }
-
-    /* Extended version string: major.minor.build (+ vendor [suffix(es)]). */
-    if (ppszVerEx)
-    {
-        *ppszVerEx = RTStrDup(VBOX_VERSION_STRING);
-        if (!*ppszVerEx)
-            return VERR_NO_STR_MEMORY;
-    }
-
-    if (ppszRev)
-    {
-        char szRev[64];
-        RTStrPrintf(szRev, sizeof(szRev), "%d", VBOX_SVN_REV);
-        *ppszRev = RTStrDup(szRev);
-        if (!*ppszRev)
-        {
-            if (ppszVer)
-            {
-                RTStrFree(*ppszVer);
-                *ppszVer = NULL;
-            }
-            return VERR_NO_STR_MEMORY;
-        }
-    }
-
-    return VINF_SUCCESS;
+        rc = RTStrDupEx(ppszVer, VBOX_VERSION_STRING_RAW);
+    if (RT_SUCCESS(rc))
+    {
+        if (ppszVerEx)
+            rc = RTStrDupEx(ppszVerEx, VBOX_VERSION_STRING);
+        if (RT_SUCCESS(rc))
+        {
+            if (ppszRev)
+            {
+#if 0
+                char szRev[64];
+                RTStrPrintf(szRev, sizeof(szRev), "%d", VBOX_SVN_REV);
+                rc = RTStrDupEx(ppszRev, szRev);
+#else
+                rc = RTStrDupEx(ppszRev, RT_XSTR(VBOX_SVN_REV));
+#endif
+            }
+            if (RT_SUCCESS(rc))
+                return VINF_SUCCESS;
+
+            /* bail out: */
+        }
+        if (ppszVerEx)
+        {
+            RTStrFree(*ppszVerEx);
+            *ppszVerEx = NULL;
+        }
+    }
+    if (ppszVer)
+    {
+        RTStrFree(*ppszVer);
+        *ppszVer = NULL;
+    }
+    return rc;
 }
 
 #ifdef RT_OS_WINDOWS
+
 /**
  * Looks up the storage path handle (registry).
@@ -85 +89 @@
  *                      vbglR3CloseAdditionsWinStoragePath().
  */
-static int vbglR3GetAdditionsWinStoragePath(PHKEY phKey)
+static int vbglR3QueryAdditionsWinStoragePath(PHKEY phKey)
 {
     /*
@@ -139 +143 @@
  * @returns IPRT status value
  * @param   hKey        Handle to close, retrieved by
- *                      vbglR3GetAdditionsWinStoragePath().
+ *                      vbglR3QueryAdditionsWinStoragePath().
  */
 static int vbglR3CloseAdditionsWinStoragePath(HKEY hKey)
@@ -173 +177 @@
     return rc;
 }
+
 
 /**
@@ -191 +196 @@
 VBGLR3DECL(int) VbglR3GetAdditionsVersion(char **ppszVer, char **ppszVerEx, char **ppszRev)
 {
+    /*
+     * Zap the return value up front.
+     */
+    if (ppszVer)
+        *ppszVer   = NULL;
+    if (ppszVerEx)
+        *ppszVerEx = NULL;
+    if (ppszRev)
+        *ppszRev   = NULL;
+
+
 #ifdef RT_OS_WINDOWS
     HKEY hKey;
-    int rc = vbglR3GetAdditionsWinStoragePath(&hKey);
+    int rc = vbglR3QueryAdditionsWinStoragePath(&hKey);
     if (RT_SUCCESS(rc))
     {
-        /* Version. */
+        /*
+         * 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;
@@ -246 +269 @@
                 rc = VERR_NO_MEMORY;
         }
-        /* Revision. */
+
+        /*
+         * Revision.
+         */
         if (ppszRev)
         {
@@ -310 +336 @@
 #ifdef RT_OS_WINDOWS
     HKEY hKey;
-    rc = vbglR3GetAdditionsWinStoragePath(&hKey);
+    rc = vbglR3QueryAdditionsWinStoragePath(&hKey);
     if (RT_SUCCESS(rc))
     {