Bug 710995 - (pvs-studio) Possible bad null-check in ShortcutResolver::Init(). False positive, no bug. Code cleanup patch. r=rstrong
authorJim Mathies <jmathies@mozilla.com>
Fri, 16 Dec 2011 08:53:07 -0600
changeset 84438 d241a415d65a793d2cd062b49a5dca38be676998
parent 84437 8a5cc33141a37978a950868c66a162cd873937b4
child 84439 7c4be303da9426c17ef8406a5670b89587ddbcc9
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs710995
milestone11.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 710995 - (pvs-studio) Possible bad null-check in ShortcutResolver::Init(). False positive, no bug. Code cleanup patch. r=rstrong
xpcom/io/nsLocalFileWin.cpp
--- a/xpcom/io/nsLocalFileWin.cpp
+++ b/xpcom/io/nsLocalFileWin.cpp
@@ -38,16 +38,17 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "mozilla/Util.h"
 
 #include "nsCOMPtr.h"
+#include "nsAutoPtr.h"
 #include "nsMemory.h"
 
 #include "nsLocalFile.h"
 #include "nsIDirectoryEnumerator.h"
 #include "nsNativeCharsetUtils.h"
 
 #include "nsISimpleEnumerator.h"
 #include "nsIComponentManager.h"
@@ -137,87 +138,61 @@ public:
     ShortcutResolver();
     // nonvirtual since we're not subclassed
     ~ShortcutResolver();
 
     nsresult Init();
     nsresult Resolve(const WCHAR* in, WCHAR* out);
 
 private:
-    Mutex         mLock;
-    IPersistFile* mPersistFile;
-    // Win 95 and 98 don't have IShellLinkW
-    IShellLinkW*  mShellLink;
+    Mutex                  mLock;
+    nsRefPtr<IPersistFile> mPersistFile;
+    nsRefPtr<IShellLinkW>  mShellLink;
 };
 
-ShortcutResolver::ShortcutResolver() : mLock("ShortcutResolver.mLock")
+ShortcutResolver::ShortcutResolver() :
+    mLock("ShortcutResolver.mLock")
 {
-    mPersistFile = nsnull;
-    mShellLink  = nsnull;
+    CoInitialize(NULL);
 }
 
 ShortcutResolver::~ShortcutResolver()
 {
-    // Release the pointer to the IPersistFile interface.
-    if (mPersistFile)
-        mPersistFile->Release();
-
-    // Release the pointer to the IShellLink interface.
-    if (mShellLink)
-        mShellLink->Release();
-
     CoUninitialize();
 }
 
 nsresult
 ShortcutResolver::Init()
 {
-    CoInitialize(NULL);  // FIX: we should probably move somewhere higher up during startup
-
-    HRESULT hres; 
-    hres = CoCreateInstance(CLSID_ShellLink,
-                            NULL,
-                            CLSCTX_INPROC_SERVER,
-                            IID_IShellLinkW,
-                            (void**)&(mShellLink));
-    if (SUCCEEDED(hres))
-    {
-        // Get a pointer to the IPersistFile interface.
-        hres = mShellLink->QueryInterface(IID_IPersistFile,
-                                          (void**)&mPersistFile);
+    // Get a pointer to the IPersistFile interface.
+    if (FAILED(CoCreateInstance(CLSID_ShellLink,
+                                NULL,
+                                CLSCTX_INPROC_SERVER,
+                                IID_IShellLinkW,
+                                getter_AddRefs(mShellLink))) ||
+        FAILED(mShellLink->QueryInterface(IID_IPersistFile,
+                                          getter_AddRefs(mPersistFile)))) {
+        mShellLink = nsnull;
+        return NS_ERROR_FAILURE;
     }
-
-    if (mPersistFile == nsnull || mShellLink == nsnull)
-        return NS_ERROR_FAILURE;
-
     return NS_OK;
 }
 
 // |out| must be an allocated buffer of size MAX_PATH
 nsresult
 ShortcutResolver::Resolve(const WCHAR* in, WCHAR* out)
 {
-    MutexAutoLock lock(mLock);
-
-    // see if we can Load the path.
-    HRESULT hres = mPersistFile->Load(in, STGM_READ);
-
-    if (FAILED(hres))
+    if (!mShellLink)
         return NS_ERROR_FAILURE;
 
-    // Resolve the link.
-    hres = mShellLink->Resolve(nsnull, SLR_NO_UI);
-
-    if (FAILED(hres))
-        return NS_ERROR_FAILURE;
-
-    // Get the path to the link target.
-    hres = mShellLink->GetPath(out, MAX_PATH, NULL, SLGP_UNCPRIORITY);
-
-    if (FAILED(hres))
+    MutexAutoLock lock(mLock);
+
+    if (FAILED(mPersistFile->Load(in, STGM_READ)) ||
+        FAILED(mShellLink->Resolve(nsnull, SLR_NO_UI)) ||
+        FAILED(mShellLink->GetPath(out, MAX_PATH, NULL, SLGP_UNCPRIORITY)))
         return NS_ERROR_FAILURE;
     return NS_OK;
 }
 
 static ShortcutResolver * gResolver = nsnull;
 
 static nsresult NS_CreateShortcutResolver()
 {