Module: wine Branch: refs/heads/master Commit: e3af1227c9debfaf2290be75201ccfc47de61270 URL: http://source.winehq.org/git/?p=wine.git;a=commit;h=e3af1227c9debfaf2290be75...
Author: Dr J A Gow J.A.Gow@furrybubble.co.uk Date: Tue Feb 21 17:03:36 2006 +0900
ole32: Fix stream ref counting. Stream methods called after parent object has been closed correctly return STG_E_REVERTED. Stream refcounting fixed. Now can safely call IStorage destructor before IStream destructor and guarantee file will be closed.
---
dlls/ole32/stg_stream.c | 74 ++++++++++++++++++++++++++++++++++++++++-- dlls/ole32/storage32.c | 53 ++++++++++++++++++++++++++++++ dlls/ole32/storage32.h | 20 +++++++++++ dlls/ole32/tests/storage32.c | 4 -- 4 files changed, 144 insertions(+), 7 deletions(-)
diff --git a/dlls/ole32/stg_stream.c b/dlls/ole32/stg_stream.c index 19b8bda..fd75ee2 100644 --- a/dlls/ole32/stg_stream.c +++ b/dlls/ole32/stg_stream.c @@ -58,8 +58,21 @@ static void StgStreamImpl_Destroy(StgStr
/* * Release the reference we are holding on the parent storage. + * IStorage_Release((IStorage*)This->parentStorage); + * + * No, don't do this. Some apps call IStorage_Release without + * calling IStream_Release first. If we grab a reference the + * file is not closed, and the app fails when it tries to + * reopen the file (Easy-PC, for example). Just inform the + * storage that we have closed the stream */ - IStorage_Release((IStorage*)This->parentStorage); + + if(This->parentStorage) { + + StorageBaseImpl_RemoveStream(This->parentStorage, This); + + } + This->parentStorage = 0;
/* @@ -447,6 +460,14 @@ static HRESULT WINAPI StgStreamImpl_Seek iface, dlibMove.u.LowPart, dwOrigin, plibNewPosition);
/* + * fail if the stream has no parent (as does windows) + */ + + if(!(This->parentStorage)) { + return STG_E_REVERTED; + } + + /* * The caller is allowed to pass in NULL as the new position return value. * If it happens, we assign it to a dynamic variable to avoid special cases * in the code below. @@ -506,6 +527,10 @@ static HRESULT WINAPI StgStreamImpl_SetS
TRACE("(%p, %ld)\n", iface, libNewSize.u.LowPart);
+ if(!This->parentStorage) { + return STG_E_REVERTED; + } + /* * As documented. */ @@ -609,6 +634,7 @@ static HRESULT WINAPI StgStreamImpl_Copy ULARGE_INTEGER* pcbRead, /* [out] */ ULARGE_INTEGER* pcbWritten) /* [out] */ { + StgStreamImpl* const This=(StgStreamImpl*)iface; HRESULT hr = S_OK; BYTE tmpBuffer[128]; ULONG bytesRead, bytesWritten, copySize; @@ -621,6 +647,11 @@ static HRESULT WINAPI StgStreamImpl_Copy /* * Sanity check */ + + if(!This->parentStorage) { + return STG_E_REVERTED; + } + if ( pstm == 0 ) return STG_E_INVALIDPOINTER;
@@ -691,6 +722,11 @@ static HRESULT WINAPI StgStreamImpl_Comm IStream* iface, DWORD grfCommitFlags) /* [in] */ { + StgStreamImpl* const This=(StgStreamImpl*)iface; + + if(!This->parentStorage) { + return STG_E_REVERTED; + } return S_OK; }
@@ -714,6 +750,12 @@ static HRESULT WINAPI StgStreamImpl_Lock ULARGE_INTEGER cb, /* [in] */ DWORD dwLockType) /* [in] */ { + StgStreamImpl* const This=(StgStreamImpl*)iface; + + if(!This->parentStorage) { + return STG_E_REVERTED; + } + FIXME("not implemented!\n"); return E_NOTIMPL; } @@ -724,6 +766,12 @@ static HRESULT WINAPI StgStreamImpl_Unlo ULARGE_INTEGER cb, /* [in] */ DWORD dwLockType) /* [in] */ { + StgStreamImpl* const This=(StgStreamImpl*)iface; + + if(!This->parentStorage) { + return STG_E_REVERTED; + } + FIXME("not implemented!\n"); return E_NOTIMPL; } @@ -747,6 +795,14 @@ static HRESULT WINAPI StgStreamImpl_Stat BOOL readSucessful;
/* + * if stream has no parent, return STG_E_REVERTED + */ + + if(!This->parentStorage) { + return STG_E_REVERTED; + } + + /* * Read the information from the property. */ readSucessful = StorageImpl_ReadProperty(This->parentStorage->ancestorStorage, @@ -791,6 +847,11 @@ static HRESULT WINAPI StgStreamImpl_Clon /* * Sanity check */ + + if(!This->parentStorage) { + return STG_E_REVERTED; + } + if ( ppstm == 0 ) return STG_E_INVALIDPOINTER;
@@ -858,12 +919,19 @@ StgStreamImpl* StgStreamImpl_Construct( newStream->lpVtbl = &StgStreamImpl_Vtbl; newStream->ref = 0;
+ newStream->parentStorage = parentStorage; + /* * We want to nail-down the reference to the storage in case the * stream out-lives the storage in the client application. + * + * -- IStorage_AddRef((IStorage*)newStream->parentStorage); + * + * No, don't do this. Some apps call IStorage_Release without + * calling IStream_Release first. If we grab a reference the + * file is not closed, and the app fails when it tries to + * reopen the file (Easy-PC, for example) */ - newStream->parentStorage = parentStorage; - IStorage_AddRef((IStorage*)newStream->parentStorage);
newStream->grfMode = grfMode; newStream->ownerProperty = ownerProperty; diff --git a/dlls/ole32/storage32.c b/dlls/ole32/storage32.c index 08084e4..ae01174 100644 --- a/dlls/ole32/storage32.c +++ b/dlls/ole32/storage32.c @@ -393,6 +393,12 @@ HRESULT WINAPI StorageBaseImpl_OpenStrea */ IStream_AddRef(*ppstm);
+ /* + * add us to the storage's list of active streams + */ + + StorageBaseImpl_AddStream(This,newStream); + res = S_OK; goto end; } @@ -979,6 +985,11 @@ HRESULT WINAPI StorageBaseImpl_CreateStr * the reference. */ IStream_AddRef(*ppstm); + + /* add us to the storage's list of active streams + */ + StorageBaseImpl_AddStream(This,newStream); + } else { @@ -1797,6 +1808,34 @@ HRESULT WINAPI StorageImpl_Stat( IStorag return result; }
+/****************************************************************************** + * Internal stream list handlers + */ + +void StorageBaseImpl_AddStream(StorageBaseImpl * stg, StgStreamImpl * strm) +{ + TRACE("Stream added (stg=%p strm=%p)\n", stg, strm); + list_add_tail(&stg->strmHead,&strm->StrmListEntry); +} + +void StorageBaseImpl_RemoveStream(StorageBaseImpl * stg, StgStreamImpl * strm) +{ + TRACE("Stream removed (stg=%p strm=%p)\n", stg,strm); + list_remove(&(strm->StrmListEntry)); +} + +void StorageBaseImpl_DeleteAll(StorageBaseImpl * stg) +{ + struct list *cur, *cur2; + StgStreamImpl *strm=NULL; + + LIST_FOR_EACH_SAFE(cur, cur2, &stg->strmHead) { + strm = LIST_ENTRY(cur,StgStreamImpl,StrmListEntry); + TRACE("Streams deleted (stg=%p strm=%p next=%p prev=%p)\n", stg,strm,cur->next,cur->prev); + strm->parentStorage = NULL; + list_remove(cur); + } +}
/********************************************************************* @@ -2256,6 +2295,12 @@ HRESULT StorageImpl_Construct( memset(This, 0, sizeof(StorageImpl));
/* + * Initialize stream list + */ + + list_init(&This->base.strmHead); + + /* * Initialize the virtual function table. */ This->base.lpVtbl = &Storage32Impl_Vtbl; @@ -2448,6 +2493,8 @@ void StorageImpl_Destroy(StorageBaseImpl StorageImpl *This = (StorageImpl*) iface; TRACE("(%p)\n", This);
+ StorageBaseImpl_DeleteAll(&This->base); + HeapFree(GetProcessHeap(), 0, This->pwcsName);
BlockChainStream_Destroy(This->smallBlockRootChain); @@ -4113,6 +4160,12 @@ StorageInternalImpl* StorageInternalImpl memset(newStorage, 0, sizeof(StorageInternalImpl));
/* + * Initialize the stream list + */ + + list_init(&newStorage->base.strmHead); + + /* * Initialize the virtual function table. */ newStorage->base.lpVtbl = &Storage32InternalImpl_Vtbl; diff --git a/dlls/ole32/storage32.h b/dlls/ole32/storage32.h index 0472524..f153021 100644 --- a/dlls/ole32/storage32.h +++ b/dlls/ole32/storage32.h @@ -37,6 +37,7 @@ #include "objbase.h" #include "winreg.h" #include "winternl.h" +#include "wine/list.h"
/* * Definitions for the file format offsets. @@ -220,6 +221,12 @@ struct StorageBaseImpl const IPropertySetStorageVtbl *pssVtbl; /* interface for adding a properties stream */
/* + * Stream tracking list + */ + + struct list strmHead; + + /* * Reference count of this object */ LONG ref; @@ -246,6 +253,13 @@ struct StorageBaseImpl DWORD openFlags; };
+/**************************************************************************** + * StorageBaseImpl stream list handlers + */ + +void StorageBaseImpl_AddStream(StorageBaseImpl * stg, StgStreamImpl * strm); +void StorageBaseImpl_RemoveStream(StorageBaseImpl * stg, StgStreamImpl * strm); +void StorageBaseImpl_DeleteAll(StorageBaseImpl * stg);
/**************************************************************************** * Storage32Impl definitions. @@ -485,6 +499,12 @@ struct StgStreamImpl * since we want to cast this to an IStream pointer */
/* + * We are an entry in the storage object's stream handler list + */ + + struct list StrmListEntry; + + /* * Reference count */ LONG ref; diff --git a/dlls/ole32/tests/storage32.c b/dlls/ole32/tests/storage32.c index b4edf13..235ca63 100644 --- a/dlls/ole32/tests/storage32.c +++ b/dlls/ole32/tests/storage32.c @@ -593,7 +593,6 @@ static void test_storage_refcount(void) r = IStorage_CreateStream(stg, stmname, STGM_SHARE_EXCLUSIVE | STGM_READWRITE, 0, 0, &stm ); ok(r==S_OK, "IStorage->CreateStream failed\n");
- todo_wine { r = IStorage_Release( stg ); ok (r == 0, "storage not released\n");
@@ -603,7 +602,6 @@ static void test_storage_refcount(void)
r = IStream_Stat( stm, &stat, STATFLAG_DEFAULT ); ok (r == STG_E_REVERTED, "stat should fail\n"); - }
r = IStream_Release(stm); ok (r == 0, "stream not released\n"); @@ -617,10 +615,8 @@ static void test_storage_refcount(void) r = IStorage_OpenStream( stg, stmname, 0, STGM_SHARE_EXCLUSIVE|STGM_READWRITE, 0, &stm ); ok(r == S_OK, "OpenStream should succeed\n");
- todo_wine { r = IStorage_Release(stg); ok(r == 0, "wrong ref count\n"); - } }
DeleteFileW(filename);