ole32: Simplify the FileMonikerImpl_BindToStorage code.

Vincent Povirk madewokherd at gmail.com
Mon Jun 30 13:57:07 CDT 2014

There are so many things wrong with the existing code.

1. StgIsStorageFile returns S_FALSE if it encounters a non-storage
file. In this case, we pass a successful result on to the caller but
don't return an IStorage pointer.
2. Calling StgIsStorageFile is unnecessary because StgOpenStorage will
tell us if the file is the wrong type. If we need to return a
different error in this case, we can just check for
3. We don't need to add a reference to the storage object because
StgOpenStorage already returns it with a reference.
4. We don't want to return early in case of success because we still
need to free filePath.

This fixes a regression where Excel 2007 fails to open .xlsx files on
network shares, because it calls BindToStorage and gets a successful
HRESULT but no IStorage interface. (The regression was caused by
049f08f4cda090189ae57d4ba58906d891ac3d4c. Previously, we failed to
create a moniker for file: url's causing Excel to give up earlier.)

I wasn't able to test what the error should be for an invalid storage
file because native keeps giving me STG_E_INVALIDFLAG, even if I set
the bind options to something that should be valid. At least it
suggests that native tries to open the file without checking it first.
-------------- next part --------------
From c447720902e6d95d05596b08d1aa47a921bfcd5d Mon Sep 17 00:00:00 2001
From: Vincent Povirk <vincent at codeweavers.com>
Date: Mon, 30 Jun 2014 13:40:02 -0500
Subject: [PATCH] ole32: Simplify the FileMonikerImpl_BindToStorage code.

 dlls/ole32/filemoniker.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/dlls/ole32/filemoniker.c b/dlls/ole32/filemoniker.c
index 06c4798..c7bbb66 100644
--- a/dlls/ole32/filemoniker.c
+++ b/dlls/ole32/filemoniker.c
@@ -605,22 +605,11 @@ FileMonikerImpl_BindToStorage(IMoniker* iface, IBindCtx* pbc, IMoniker* pmkToLef
             /* get the file name */
-            /* verify if the file contains a storage object */
-            res=StgIsStorageFile(filePath);
+            res=StgOpenStorage(filePath,NULL,STGM_READWRITE|STGM_SHARE_DENY_WRITE,NULL,0,&pstg);
-            if(res==S_OK){
+            if (SUCCEEDED(res))
+                *ppvObject=pstg;
-                res=StgOpenStorage(filePath,NULL,STGM_READWRITE|STGM_SHARE_DENY_WRITE,NULL,0,&pstg);
-                if (SUCCEEDED(res)){
-                    *ppvObject=pstg;
-                    IStorage_AddRef(pstg);
-                    return res;
-                }
-            }

More information about the wine-patches mailing list