[2/2] ole32: Use grfLocksSupported to decide when LockRegion is implemented.
Vincent Povirk
madewokherd at gmail.com
Mon Jul 6 15:44:06 CDT 2015
Following up on Dmitry's patch, testing shows native uses
ILockBytes::Stat to determine which lock types are supported, and only
calls the function if there are some. It also doesn't appear to give
STG_E_INVALIDFUNCTION special treatment, so I guess looking for an
"unimplmented" hresult was completely wrong.
I had to add an internal flag for read locks, since support for
dwLockType of 0 can't be expressed in this way. This should be OK
because we won't ever give a custom ILockBytes that flag.
-------------- next part --------------
From 7cc69c8b5001f1e41ee291110ae1b3ebc6bea793 Mon Sep 17 00:00:00 2001
From: Vincent Povirk <vincent at codeweavers.com>
Date: Mon, 6 Jul 2015 15:11:16 -0500
Subject: [PATCH 2/2] ole32: Use grfLocksSupported to decide when LockRegion is
implemented.
---
dlls/ole32/filelockbytes.c | 2 ++
dlls/ole32/storage32.c | 75 +++++++++++++++++++++++++++++---------------
dlls/ole32/storage32.h | 5 +++
dlls/ole32/tests/storage32.c | 34 ++++++++++++++++++--
4 files changed, 88 insertions(+), 28 deletions(-)
diff --git a/dlls/ole32/filelockbytes.c b/dlls/ole32/filelockbytes.c
index 594da1b..c5cb45f 100644
--- a/dlls/ole32/filelockbytes.c
+++ b/dlls/ole32/filelockbytes.c
@@ -386,6 +386,8 @@ static HRESULT WINAPI FileLockBytesImpl_Stat(ILockBytes* iface,
pstatstg->cbSize.u.LowPart = GetFileSize(This->hfile, &pstatstg->cbSize.u.HighPart);
/* FIXME: If the implementation is exported, we'll need to set other fields. */
+ pstatstg->grfLocksSupported = LOCK_EXCLUSIVE|LOCK_ONLYONCE|WINE_LOCK_READ;
+
return S_OK;
}
diff --git a/dlls/ole32/storage32.c b/dlls/ole32/storage32.c
index aa53758..4b6f630 100644
--- a/dlls/ole32/storage32.c
+++ b/dlls/ole32/storage32.c
@@ -4822,9 +4822,31 @@ static HRESULT StorageImpl_SetTransactionSig(StorageBaseImpl *base,
return S_OK;
}
+static HRESULT StorageImpl_LockRegion(StorageImpl *This, ULARGE_INTEGER offset,
+ ULARGE_INTEGER cb, DWORD dwLockType, BOOL *supported)
+{
+ if ((dwLockType & This->locks_supported) == 0)
+ {
+ if (supported) *supported = FALSE;
+ return S_OK;
+ }
+
+ if (supported) *supported = TRUE;
+ return ILockBytes_LockRegion(This->lockBytes, offset, cb, dwLockType);
+}
+
+static HRESULT StorageImpl_UnlockRegion(StorageImpl *This, ULARGE_INTEGER offset,
+ ULARGE_INTEGER cb, DWORD dwLockType)
+{
+ if ((dwLockType & This->locks_supported) == 0)
+ return S_OK;
+
+ return ILockBytes_UnlockRegion(This->lockBytes, offset, cb, dwLockType);
+}
+
/* Internal function */
static HRESULT StorageImpl_LockRegionSync(StorageImpl *This, ULARGE_INTEGER offset,
- ULARGE_INTEGER cb, DWORD dwLockType)
+ ULARGE_INTEGER cb, DWORD dwLockType, BOOL *supported)
{
HRESULT hr;
int delay = 0;
@@ -4837,7 +4859,7 @@ static HRESULT StorageImpl_LockRegionSync(StorageImpl *This, ULARGE_INTEGER offs
do
{
- hr = ILockBytes_LockRegion(This->lockBytes, offset, cb, dwLockType);
+ hr = StorageImpl_LockRegion(This, offset, cb, dwLockType, supported);
if (hr == STG_E_ACCESSDENIED || hr == STG_E_LOCKVIOLATION)
{
@@ -4857,17 +4879,12 @@ static HRESULT StorageImpl_LockRegionSync(StorageImpl *This, ULARGE_INTEGER offs
*
* This can collide with another attempt to open the file in
* exclusive mode, but it's unlikely, and someone would fail anyway. */
- hr = ILockBytes_LockRegion(This->lockBytes, sanity_offset, sanity_cb, 0);
+ hr = StorageImpl_LockRegion(This, sanity_offset, sanity_cb, WINE_LOCK_READ, NULL);
if (hr == STG_E_ACCESSDENIED || hr == STG_E_LOCKVIOLATION)
break;
- if (hr == STG_E_INVALIDFUNCTION)
- {
- /* ignore this, lockbytes might support dwLockType but not 0 */
- hr = STG_E_ACCESSDENIED;
- }
if (SUCCEEDED(hr))
{
- ILockBytes_UnlockRegion(This->lockBytes, sanity_offset, sanity_cb, 0);
+ StorageImpl_UnlockRegion(This, sanity_offset, sanity_cb, WINE_LOCK_READ);
hr = STG_E_ACCESSDENIED;
}
@@ -4900,10 +4917,7 @@ static HRESULT StorageImpl_LockTransaction(StorageBaseImpl *base, BOOL write)
cb.QuadPart = 1;
}
- hr = StorageImpl_LockRegionSync(This, offset, cb, LOCK_ONLYONCE);
-
- if (hr == STG_E_INVALIDFUNCTION)
- hr = S_OK;
+ hr = StorageImpl_LockRegionSync(This, offset, cb, LOCK_ONLYONCE, NULL);
return hr;
}
@@ -4925,10 +4939,7 @@ static HRESULT StorageImpl_UnlockTransaction(StorageBaseImpl *base, BOOL write)
cb.QuadPart = 1;
}
- hr = ILockBytes_UnlockRegion(This->lockBytes, offset, cb, LOCK_ONLYONCE);
-
- if (hr == STG_E_INVALIDFUNCTION)
- hr = S_OK;
+ hr = StorageImpl_UnlockRegion(This, offset, cb, LOCK_ONLYONCE);
return hr;
}
@@ -4955,10 +4966,10 @@ static HRESULT StorageImpl_CheckLockRange(StorageImpl *This, ULONG start,
offset.QuadPart = start;
cb.QuadPart = 1 + end - start;
- hr = ILockBytes_LockRegion(This->lockBytes, offset, cb, LOCK_ONLYONCE);
- if (SUCCEEDED(hr)) ILockBytes_UnlockRegion(This->lockBytes, offset, cb, LOCK_ONLYONCE);
+ hr = StorageImpl_LockRegion(This, offset, cb, LOCK_ONLYONCE, NULL);
+ if (SUCCEEDED(hr)) StorageImpl_UnlockRegion(This, offset, cb, LOCK_ONLYONCE);
- if (hr == STG_E_ACCESSDENIED || hr == STG_E_LOCKVIOLATION)
+ if (FAILED(hr))
return fail_hr;
else
return S_OK;
@@ -4975,7 +4986,7 @@ static HRESULT StorageImpl_LockOne(StorageImpl *This, ULONG start, ULONG end)
for (i=start; i<=end; i++)
{
offset.QuadPart = i;
- hr = ILockBytes_LockRegion(This->lockBytes, offset, cb, LOCK_ONLYONCE);
+ hr = StorageImpl_LockRegion(This, offset, cb, LOCK_ONLYONCE, NULL);
if (hr != STG_E_ACCESSDENIED && hr != STG_E_LOCKVIOLATION)
break;
}
@@ -5001,6 +5012,7 @@ static HRESULT StorageImpl_GrabLocks(StorageImpl *This, DWORD openFlags)
ULARGE_INTEGER offset;
ULARGE_INTEGER cb;
DWORD share_mode = STGM_SHARE_MODE(openFlags);
+ BOOL supported;
if (openFlags & STGM_NOSNAPSHOT)
{
@@ -5012,10 +5024,10 @@ static HRESULT StorageImpl_GrabLocks(StorageImpl *This, DWORD openFlags)
/* Wrap all other locking inside a single lock so we can check ranges safely */
offset.QuadPart = RANGELOCK_CHECKLOCKS;
cb.QuadPart = 1;
- hr = StorageImpl_LockRegionSync(This, offset, cb, LOCK_ONLYONCE);
+ hr = StorageImpl_LockRegionSync(This, offset, cb, LOCK_ONLYONCE, &supported);
/* If the ILockBytes doesn't support locking that's ok. */
- if (hr == STG_E_INVALIDFUNCTION || hr == STG_E_UNIMPLEMENTEDFUNCTION) return S_OK;
+ if (!supported) return S_OK;
else if (FAILED(hr)) return hr;
hr = S_OK;
@@ -5069,7 +5081,7 @@ static HRESULT StorageImpl_GrabLocks(StorageImpl *This, DWORD openFlags)
offset.QuadPart = RANGELOCK_CHECKLOCKS;
cb.QuadPart = 1;
- ILockBytes_UnlockRegion(This->lockBytes, offset, cb, LOCK_ONLYONCE);
+ StorageImpl_UnlockRegion(This, offset, cb, LOCK_ONLYONCE);
return hr;
}
@@ -5134,7 +5146,7 @@ static void StorageImpl_Destroy(StorageBaseImpl* iface)
if (This->locked_bytes[i] != 0)
{
offset.QuadPart = This->locked_bytes[i];
- ILockBytes_UnlockRegion(This->lockBytes, offset, cb, LOCK_ONLYONCE);
+ StorageImpl_UnlockRegion(This, offset, cb, LOCK_ONLYONCE);
}
}
@@ -5201,7 +5213,8 @@ static HRESULT StorageImpl_Construct(
StorageImpl** result)
{
StorageImpl* This;
- HRESULT hr = S_OK;
+ HRESULT hr = S_OK;
+ STATSTG stat;
if ( FAILED( validateSTGM(openFlags) ))
return STG_E_INVALIDFLAG;
@@ -5247,7 +5260,17 @@ static HRESULT StorageImpl_Construct(
}
if (SUCCEEDED(hr))
+ hr = ILockBytes_Stat(This->lockBytes, &stat, STATFLAG_NONAME);
+
+ if (SUCCEEDED(hr))
+ {
+ This->locks_supported = stat.grfLocksSupported;
+ if (!hFile)
+ /* Don't try to use wine-internal locking flag with custom ILockBytes */
+ This->locks_supported &= ~WINE_LOCK_READ;
+
hr = StorageImpl_GrabLocks(This, openFlags);
+ }
if (SUCCEEDED(hr))
hr = StorageImpl_Refresh(This, TRUE, create);
diff --git a/dlls/ole32/storage32.h b/dlls/ole32/storage32.h
index 59d52ef..4fcfd9c 100644
--- a/dlls/ole32/storage32.h
+++ b/dlls/ole32/storage32.h
@@ -411,6 +411,8 @@ struct StorageImpl
BlockChainStream* blockChainCache[BLOCKCHAIN_CACHE_SIZE];
UINT blockChainToEvict;
+ ULONG locks_supported;
+
ILockBytes* lockBytes;
ULONG locked_bytes[8];
@@ -517,6 +519,9 @@ StgStreamImpl* StgStreamImpl_Construct(
#define RANGELOCK_FIRST RANGELOCK_UNK1_FIRST
#define RANGELOCK_LAST RANGELOCK_UNK2_LAST
+/* internal value for LockRegion/UnlockRegion */
+#define WINE_LOCK_READ 0x80000000
+
/******************************************************************************
* Endian conversion macros
diff --git a/dlls/ole32/tests/storage32.c b/dlls/ole32/tests/storage32.c
index 63a803a..fe5ddbc 100644
--- a/dlls/ole32/tests/storage32.c
+++ b/dlls/ole32/tests/storage32.c
@@ -63,6 +63,9 @@ typedef struct TestLockBytes {
BYTE* contents;
ULONG size;
ULONG buffer_size;
+ HRESULT lock_hr;
+ ULONG locks_supported;
+ ULONG lock_called;
} TestLockBytes;
static inline TestLockBytes *impl_from_ILockBytes(ILockBytes *iface)
@@ -181,13 +184,16 @@ static HRESULT WINAPI TestLockBytes_SetSize(ILockBytes *iface,
static HRESULT WINAPI TestLockBytes_LockRegion(ILockBytes *iface,
ULARGE_INTEGER libOffset, ULARGE_INTEGER cb, DWORD dwLockType)
{
- return S_OK;
+ TestLockBytes *This = impl_from_ILockBytes(iface);
+ This->lock_called++;
+ return This->lock_hr;
}
static HRESULT WINAPI TestLockBytes_UnlockRegion(ILockBytes *iface,
ULARGE_INTEGER libOffset, ULARGE_INTEGER cb, DWORD dwLockType)
{
- return S_OK;
+ TestLockBytes *This = impl_from_ILockBytes(iface);
+ return This->lock_hr;
}
static HRESULT WINAPI TestLockBytes_Stat(ILockBytes *iface,
@@ -209,6 +215,7 @@ static HRESULT WINAPI TestLockBytes_Stat(ILockBytes *iface,
pstatstg->type = STGTY_LOCKBYTES;
pstatstg->cbSize.QuadPart = This->size;
+ pstatstg->grfLocksSupported = This->locks_supported;
return S_OK;
}
@@ -3855,6 +3862,29 @@ static void test_custom_lockbytes(void)
IStorage_Release(stg);
+ ok(!lockbytes->lock_called, "unexpected call to LockRegion\n");
+
+ lockbytes->locks_supported = LOCK_WRITE|LOCK_EXCLUSIVE|LOCK_ONLYONCE;
+
+ hr = StgCreateDocfileOnILockBytes(&lockbytes->ILockBytes_iface, STGM_CREATE|STGM_READWRITE|STGM_TRANSACTED, 0, &stg);
+ ok(hr==S_OK, "StgCreateDocfileOnILockBytes failed %x\n", hr);
+
+ hr = IStorage_CreateStream(stg, stmname, STGM_SHARE_EXCLUSIVE|STGM_READWRITE, 0, 0, &stm);
+ ok(hr==S_OK, "IStorage_CreateStream failed %x\n", hr);
+
+ IStream_Release(stm);
+
+ hr = IStorage_Commit(stg, 0);
+
+ IStorage_Release(stg);
+
+ ok(lockbytes->lock_called, "expected LockRegion to be called\n");
+
+ lockbytes->lock_hr = STG_E_INVALIDFUNCTION;
+
+ hr = StgCreateDocfileOnILockBytes(&lockbytes->ILockBytes_iface, STGM_CREATE|STGM_READWRITE|STGM_TRANSACTED, 0, &stg);
+ ok(hr==STG_E_INVALIDFUNCTION, "StgCreateDocfileOnILockBytes failed %x\n", hr);
+
DeleteTestLockBytes(lockbytes);
}
--
2.1.4
More information about the wine-patches
mailing list