Bug 710995 - (pvs-studio) Possible bad null-check in ShortcutResolver::Init(). False positive, no bug. Code cleanup patch. r=rstrong

xpcom/io/nsLocalFileWin.cpp

@@ -43,6 +43,7 @@
 #include "mozilla/Util.h"
 
 #include "nsCOMPtr.h"
+#include "nsAutoPtr.h"
 #include "nsMemory.h"
 
 #include "nsLocalFile.h"
@@ -142,52 +143,36 @@ public:
     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);
-    }
-
-    if (mPersistFile == nsnull || mShellLink == nsnull)
+    // 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;
-
+    }
     return NS_OK;
 }
 
@@ -195,24 +180,14 @@ ShortcutResolver::Init()
 nsresult
 ShortcutResolver::Resolve(const WCHAR* in, WCHAR* out)
 {
+    if (!mShellLink)
+        return NS_ERROR_FAILURE;
+
     MutexAutoLock lock(mLock);
 
-    // see if we can Load the path.
-    HRESULT hres = mPersistFile->Load(in, STGM_READ);
-
-    if (FAILED(hres))
-        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))
+    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;
 }