Changeset 24384 in vbox

Timestamp:

Nov 5, 2009 2:06:22 PM (15 years ago)

Author:

vboxsync

Message:

VbglR3: Various fixes based on review.

Location:

trunk/src/VBox/Additions/common/VBoxGuestLib

Files:

• VBoxGuestR3LibHostVersion.cpp (modified) (2 diffs)
• VBoxGuestR3LibMisc.cpp (modified) (6 diffs)

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

@@ -24 +24 @@
 *   Header Files                                                               *
 *******************************************************************************/
-#include <stdlib.h>
-#include <stdio.h>
+#include <stdio.h>          /* Required for sscanf */
 #include <iprt/string.h>
 #include <VBox/log.h>
@@ -114 +113 @@
         /* Only don't do the check if we have a valid "0" in it */
         if (   *pszCheckHostVersion
-            && atoi(pszCheckHostVersion) == 0) /** @todo r=bird: don't use atoi, use RTStrToXX. avoid std*.h! */
+            && RTStrToInt16(pszCheckHostVersion) == 0) /* Either string conversion failed or we really did disable it */
         {
             LogRel(("No host version update check performed (disabled).\n"));

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

@@ -238 +238 @@
 VBGLR3DECL(int) VbglR3GetAdditionsVersion(char **ppszVer, char **ppszRev)
 {
-#ifdef RT_OS_WINDOWS
+# ifdef RT_OS_WINDOWS
     /*
      * Try get the *installed* version first.
      */
-    HKEY hKey;
+    HKEY hKey = NULL;
     LONG r;
 
@@ -272 +272 @@
     if (r == ERROR_SUCCESS)
     {
-/** @todo r=bird: If anything fails here, this code will end up returning
- *        rc=VINF_SUCCESS and uninitialized output pointers.  It will also
- *        leak memory in some cases.  Iff the value type isn't string,
- *        garbage is returned.
- *
- *        RTMemAlloc shall be freed by RTMemFree not RTStrFree.  Don't ever mix
- *        because it will blow up in an annoying way when using the eletrical
- *        fences and stuff.  Use a temporary buffer and RTStrDupEx.
- */
         /* Version. */
         DWORD dwType;
@@ -292 +283 @@
                 r = RegQueryValueEx(hKey, "Version", NULL, &dwType, (BYTE*)(LPCTSTR)pszTmp, &dwSize);
                 if (r == ERROR_SUCCESS)
-                    rc = RTStrDupEx(ppszVer, pszTmp);
+                {
+                    if (dwType == REG_SZ)
+                        rc = RTStrDupEx(ppszVer, pszTmp);
+                    else
+                        rc = VERR_INVALID_PARAMETER;
+                }
                 else
+                {
                     rc = RTErrConvertFromNtStatus(r);
+                }
                 RTMemFree(pszTmp);
             }
@@ -309 +307 @@
                 r = RegQueryValueEx(hKey, "Revision", NULL, &dwType, (BYTE*)(LPCTSTR)pszTmp, &dwSize);
                 if (r == ERROR_SUCCESS)
-                    rc = RTStrDupEx(ppszRev, pszTmp);
+                {
+                    if (dwType == REG_SZ)
+                        rc = RTStrDupEx(ppszRev, pszTmp);
+                    else
+                        rc = VERR_INVALID_PARAMETER;
+                }
                 else
+                {
                     rc = RTErrConvertFromNtStatus(r);
+                }
                 RTMemFree(pszTmp);
             }
@@ -317 +322 @@
                 rc = VERR_NO_MEMORY;
         }
+        if (hKey != NULL)
+            RegCloseKey(hKey);
     }
     else
@@ -326 +333 @@
         rc = vbglR3GetAdditionsCompileTimeVersion(ppszVer, ppszRev);
     }
-    if (hKey != NULL) /** @todo r=bird: This looks kind of wrong for the failure case... */
-        RegCloseKey(hKey);
-    return rc;
-
-#else /* !RT_OS_WINDOWS */
+    return rc;
+
+# else /* !RT_OS_WINDOWS */
     /*
      * On non-Windows platforms just return the compile-time version string.
      */
     return vbglR3GetAdditionsCompileTimeVersion(ppszVer, ppszRev);
-#endif /* !RT_OS_WINDOWS */
-}
+# endif /* !RT_OS_WINDOWS */
+}