[PATCH 1/4] ole32: Add support for loading enhmetafiles from presentation and contents streams to data cache.

[Sergio Gómez Del Real]

Signed-off-by: Sergio Gómez Del Real 
---
 dlls/ole32/datacache.c | 97 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 73 insertions(+), 24 deletions(-)

diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
index 325a98b33b..e511eac23e 100644
--- a/dlls/ole32/datacache.c
+++ b/dlls/ole32/datacache.c
@@ -547,6 +547,30 @@ static HRESULT open_pres_stream( IStorage *stg, int stream_number, IStream **stm
     return IStorage_OpenStream( stg, name, NULL, STGM_READ | STGM_SHARE_EXCLUSIVE, 0, stm );
 }

+static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
+{
+    METAFILEPICT *pict;
+    HRESULT hr = E_FAIL;
+    UINT size;
+    void *bits;
+
+    if (!(pict = GlobalLock( data ))) return hr;
+
+    size = GetMetaFileBitsEx( pict->hMF, 0, NULL );
+    if ((bits = HeapAlloc( GetProcessHeap(), 0, size )))
+    {
+        GetMetaFileBitsEx( pict->hMF, size, bits );
+        med->u.hEnhMetaFile = SetWinMetaFileBits( size, bits, NULL, pict );
+        HeapFree( GetProcessHeap(), 0, bits );
+        med->tymed = TYMED_ENHMF;
+        med->pUnkForRelease = NULL;
+        hr = S_OK;
+    }
+
+    GlobalUnlock( data );
+    return hr;
+}
+
 static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
 {
     HRESULT hr;
@@ -701,7 +725,52 @@ fail:
     GlobalUnlock( hglobal );
     GlobalFree( hglobal );
     return E_FAIL;
+}
+
+static HRESULT load_emf( DataCacheEntry *cache_entry, IStream *stm )
+{
+
+    if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
+    {
+        HMETAFILEPICT hmfpict;
+        METAFILEPICT *mfpict;
+
+        load_mf_pict(cache_entry, stm);
+        hmfpict = cache_entry->stgmedium.u.hMetaFilePict;
+        synthesize_emf(hmfpict, &cache_entry->stgmedium);
+        mfpict = GlobalLock(hmfpict);
+        DeleteMetaFile(mfpict->hMF);
+        GlobalFree(hmfpict);
+    }
+    else
+    {
+        HRESULT hr;
+        STATSTG stat;
+        void *data;
+        DWORD size_bits;
+        ULONG read;
+
+        hr = IStream_Stat( stm, &stat, STATFLAG_NONAME );
+        if (FAILED( hr )) return hr;

+        data = HeapAlloc( GetProcessHeap(), 0, stat.cbSize.u.LowPart );
+        if (!data) return E_OUTOFMEMORY;
+
+        hr = IStream_Read( stm, data, stat.cbSize.u.LowPart, &read );
+        if (hr != S_OK || read != stat.cbSize.u.LowPart) hr = E_FAIL;
+
+        size_bits = read - sizeof(DWORD) - sizeof(ENHMETAHEADER);
+        if (size_bits <= 0)
+        {
+            HeapFree( GetProcessHeap(), 0, data );
+            return E_FAIL;
+        }
+        cache_entry->stgmedium.u.hEnhMetaFile = SetEnhMetaFileBits( size_bits, (BYTE *)data + (read - size_bits) );
+        cache_entry->stgmedium.tymed = TYMED_ENHMF;
+        cache_entry->stgmedium.pUnkForRelease = NULL;
+    }
+
+    return S_OK;
 }

/************************************************************************
@@ -736,6 +805,10 @@ static HRESULT DataCacheEntry_LoadData(DataCacheEntry *cache_entry, IStorage *st
         hr = load_dib( cache_entry, stm );
         break;

+    case CF_ENHMETAFILE:
+        hr = load_emf( cache_entry, stm );
+        break;
+
     default:
         FIXME( "Unimplemented clip format %x\n", cache_entry->fmtetc.cfFormat );
         hr = E_NOTIMPL;
@@ -1118,30 +1191,6 @@ static HRESULT synthesize_bitmap( HGLOBAL dib, STGMEDIUM *med )
     return hr;
 }

-static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
-{
-    METAFILEPICT *pict;
-    HRESULT hr = E_FAIL;
-    UINT size;
-    void *bits;
-
-    if (!(pict = GlobalLock( data ))) return hr;
-
-    size = GetMetaFileBitsEx( pict->hMF, 0, NULL );
-    if ((bits = HeapAlloc( GetProcessHeap(), 0, size )))
-    {
-        GetMetaFileBitsEx( pict->hMF, size, bits );
-        med->u.hEnhMetaFile = SetWinMetaFileBits( size, bits, NULL, pict );
-        HeapFree( GetProcessHeap(), 0, bits );
-        med->tymed = TYMED_ENHMF;
-        med->pUnkForRelease = NULL;
-        hr = S_OK;
-    }
-
-    GlobalUnlock( data );
-    return hr;
-}
-
 static HRESULT DataCacheEntry_SetData(DataCacheEntry *cache_entry,
                                       const FORMATETC *formatetc,
                                       STGMEDIUM *stgmedium,

[PATCH 2/4] ole32: Add support for loading dibs from presentation streams to data cache.

[Sergio Gómez Del Real]

Signed-off-by: Sergio Gómez Del Real 
---
 dlls/ole32/datacache.c | 62 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
index e511eac23e..673f690934 100644
--- a/dlls/ole32/datacache.c
+++ b/dlls/ole32/datacache.c
@@ -571,6 +571,18 @@ static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
     return hr;
 }

+static HRESULT read_pres_header( IStream *stm, void *pres_hdr )
+{
+    ULONG read, pres_hdr_size;
+    HRESULT hr;
+
+    pres_hdr_size = sizeof(PresentationDataHeader) + sizeof(DWORD) * 2;
+    hr = IStream_Read( stm, pres_hdr, pres_hdr_size, &read );
+    if (hr != S_OK || read != pres_hdr_size) return E_FAIL;
+
+    return S_OK;
+}
+
 static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
 {
     HRESULT hr;
@@ -650,32 +662,42 @@ static HRESULT load_dib( DataCacheEntry *cache_entry, IStream *stm )
     void *dib;
     HGLOBAL hglobal;
     ULONG read, info_size, bi_size;
-    BITMAPFILEHEADER file;
     BITMAPINFOHEADER *info;
-
-    if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
-    {
-        FIXME( "Unimplemented for presentation stream\n" );
-        return E_FAIL;
-    }
+    /* only standard clipformats */
+    struct {
+        DWORD cf[2];
+        PresentationDataHeader pres;
+    } pres_hdr;
+    BITMAPFILEHEADER file;

hr = IStream_Stat( stm, &stat, STATFLAG_NONAME );
     if (FAILED( hr )) return hr;

-    if (stat.cbSize.QuadPart < sizeof(file) + sizeof(DWORD)) return E_FAIL;
-    hr = IStream_Read( stm, &file, sizeof(file), &read );
-    if (hr != S_OK || read != sizeof(file)) return E_FAIL;
-    stat.cbSize.QuadPart -= sizeof(file);
+    if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
+    {
+        if (stat.cbSize.QuadPart < sizeof(pres_hdr)) return E_FAIL;
+        hr = read_pres_header( stm, &pres_hdr );
+        if (FAILED( hr )) return hr;
+        stat.cbSize.QuadPart -= sizeof(pres_hdr);
+    }
+    else
+    {
+        if (stat.cbSize.QuadPart < sizeof(BITMAPFILEHEADER)) return E_FAIL;
+        hr = IStream_Read( stm, &file, sizeof(BITMAPFILEHEADER), &read );
+        stat.cbSize.QuadPart -= sizeof(BITMAPFILEHEADER);
+    }

hglobal = GlobalAlloc( GMEM_MOVEABLE, stat.cbSize.u.LowPart );
     if (!hglobal) return E_OUTOFMEMORY;
     dib = GlobalLock( hglobal );

+    /* read first DWORD of BITMAPINFOHEADER */
     hr = IStream_Read( stm, dib, sizeof(DWORD), &read );
     if (hr != S_OK || read != sizeof(DWORD)) goto fail;
     bi_size = *(DWORD *)dib;
     if (stat.cbSize.QuadPart < bi_size) goto fail;

+    /* read rest of BITMAPINFOHEADER */
     hr = IStream_Read( stm, (char *)dib + sizeof(DWORD), bi_size - sizeof(DWORD), &read );
     if (hr != S_OK || read != bi_size - sizeof(DWORD)) goto fail;

@@ -688,15 +710,19 @@ static HRESULT load_dib( DataCacheEntry *cache_entry, IStream *stm )
     }
     stat.cbSize.QuadPart -= info_size;

-    if (file.bfOffBits)
+    /* set Stream pointer to beginning of bitmap bits */
+    if (cache_entry->load_stream_num == STREAM_NUMBER_CONTENTS)
     {
-        LARGE_INTEGER skip;
+        if (file.bfOffBits)
+        {
+            LARGE_INTEGER skip;

-        skip.QuadPart = file.bfOffBits - sizeof(file) - info_size;
-        if (stat.cbSize.QuadPart < skip.QuadPart) goto fail;
-        hr = IStream_Seek( stm, skip, STREAM_SEEK_CUR, NULL );
-        if (hr != S_OK) goto fail;
-        stat.cbSize.QuadPart -= skip.QuadPart;
+            skip.QuadPart = file.bfOffBits - sizeof(file) - info_size;
+            if (stat.cbSize.QuadPart < skip.QuadPart) goto fail;
+            hr = IStream_Seek( stm, skip, STREAM_SEEK_CUR, NULL );
+            if (hr != S_OK) goto fail;
+            stat.cbSize.QuadPart -= skip.QuadPart;
+        }
     }

hr = IStream_Read( stm, (char *)dib + info_size, stat.cbSize.u.LowPart, &read );

[PATCH 2/4] ole32: Add support for loading dibs from presentation streams to data cache.

[Huw Davies]

On Tue, Mar 27, 2018 at 11:39:59AM -0500, Sergio Gómez Del Real wrote:
> Signed-off-by: Sergio Gómez Del Real
>
> dlls/ole32/datacache.c | 62 +++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
> index e511eac23e..673f690934 100644
> --- a/dlls/ole32/datacache.c
> +++ b/dlls/ole32/datacache.c
> @@ -571,6 +571,18 @@ static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
>      return hr;
>  }
>
> +static HRESULT read_pres_header( IStream *stm, void *pres_hdr )
> +{

Having a void * here rings alarm bells.  You probably want something like

read_pres_header( IStream *, DWORD *clip_format, PresentationDataHeader *)
> - ULONG read, pres_hdr_size;
> - HRESULT hr;
> -
> - pres_hdr_size = sizeof(PresentationDataHeader) + sizeof(DWORD) * 2;
> - hr = IStream_Read( stm, pres_hdr, pres_hdr_size, &read );
> - if (hr != S_OK || read != pres_hdr_size) return E_FAIL;
> -
> - return S_OK;
>
> +}
> -
>
> static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
>  {
>      HRESULT hr;
> @@ -650,32 +662,42 @@ static HRESULT load_dib( DataCacheEntry *cache_entry, IStream *stm )
>      void *dib;
>      HGLOBAL hglobal;
>      ULONG read, info_size, bi_size;
> - BITMAPFILEHEADER file;
> BITMAPINFOHEADER *info;
> -
> - if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
> - {
> -
>    FIXME( "Unimplemented for presentation stream\n" );
> -
>    return E_FAIL;
> - }
> -
>
> /* only standard clipformats */
> -
>
> struct {
> -
>    DWORD cf[2];
> -
>    PresentationDataHeader pres;
> -
>
> } pres_hdr;
> -
>
> BITMAPFILEHEADER file;
>
> hr = IStream_Stat( stm, &stat, STATFLAG_NONAME );
> if (FAILED( hr )) return hr;
> - if (stat.cbSize.QuadPart < sizeof(file) + sizeof(DWORD)) return E_FAIL;
> - hr = IStream_Read( stm, &file, sizeof(file), &read );
> - if (hr != S_OK || read != sizeof(file)) return E_FAIL;
> - stat.cbSize.QuadPart -= sizeof(file);
> -
>
> if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
> -
>
> {
> -
>    if (stat.cbSize.QuadPart < sizeof(pres_hdr)) return E_FAIL;
> -
>    hr = read_pres_header( stm, &pres_hdr );
> -
>    if (FAILED( hr )) return hr;
> -
>    stat.cbSize.QuadPart -= sizeof(pres_hdr);
> -
>
> }
> -
>
> else
> -
>
> {
> -
>    if (stat.cbSize.QuadPart < sizeof(BITMAPFILEHEADER)) return E_FAIL;
> -
>    hr = IStream_Read( stm, &file, sizeof(BITMAPFILEHEADER), &read );
> -
>    stat.cbSize.QuadPart -= sizeof(BITMAPFILEHEADER);
> -
>
> }
>
> hglobal = GlobalAlloc( GMEM_MOVEABLE, stat.cbSize.u.LowPart );
> if (!hglobal) return E_OUTOFMEMORY;
> dib = GlobalLock( hglobal );
> -
>
> /* read first DWORD of BITMAPINFOHEADER */
> hr = IStream_Read( stm, dib, sizeof(DWORD), &read );
> if (hr != S_OK || read != sizeof(DWORD)) goto fail;
> bi_size = *(DWORD *)dib;
> if (stat.cbSize.QuadPart < bi_size) goto fail;
> -
>
> /* read rest of BITMAPINFOHEADER */
> hr = IStream_Read( stm, (char *)dib + sizeof(DWORD), bi_size - sizeof(DWORD), &read );
> if (hr != S_OK || read != bi_size - sizeof(DWORD)) goto fail;
>
> @@ -688,15 +710,19 @@ static HRESULT load_dib( DataCacheEntry *cache_entry, IStream *stm )
>      }
>      stat.cbSize.QuadPart -= info_size;
> - if (file.bfOffBits)
> - /* set Stream pointer to beginning of bitmap bits */
> - if (cache_entry->load_stream_num == STREAM_NUMBER_CONTENTS)
> {
> -
>    LARGE_INTEGER skip;
> -
>    if (file.bfOffBits)

There's not need for two if()s here just combine them with an &&.

As a bonus the indentation of the block won't change, so your patch
will be smaller.
> -
>    {
> -
>        LARGE_INTEGER skip;
>
> -
>    skip.QuadPart = file.bfOffBits - sizeof(file) - info_size;
> -
>    if (stat.cbSize.QuadPart < skip.QuadPart) goto fail;
> -
>    hr = IStream_Seek( stm, skip, STREAM_SEEK_CUR, NULL );
> -
>    if (hr != S_OK) goto fail;
> -
>    stat.cbSize.QuadPart -= skip.QuadPart;
> -
>        skip.QuadPart = file.bfOffBits - sizeof(file) - info_size;
> -
>        if (stat.cbSize.QuadPart < skip.QuadPart) goto fail;
> -
>        hr = IStream_Seek( stm, skip, STREAM_SEEK_CUR, NULL );
> -
>        if (hr != S_OK) goto fail;
> -
>        stat.cbSize.QuadPart -= skip.QuadPart;
> -
>    }
>
> }
>
> hr = IStream_Read( stm, (char *)dib + info_size, stat.cbSize.u.LowPart, &read );

This function is becoming hard to follow, let's see if those changes
help or whether we need to be more surgical.

[PATCH 3/4] ole32: Add support for loading wmf from contents streams to data cache.

[Sergio Gómez Del Real]

Signed-off-by: Sergio Gómez Del Real 
---
 dlls/ole32/datacache.c | 71 +++++++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
index 673f690934..5a56eb76b5 100644
--- a/dlls/ole32/datacache.c
+++ b/dlls/ole32/datacache.c
@@ -570,6 +570,17 @@ static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
     GlobalUnlock( data );
     return hr;
 }
+#include <pshpack2.h>
+struct meta_placeable
+{
+    DWORD key;
+    WORD hwmf;
+    WORD bounding_box[4];
+    WORD inch;
+    DWORD reserved;
+    WORD checksum;
+};
+#include <poppack.h>

static HRESULT read_pres_header( IStream *stm, void *pres_hdr )
 {
@@ -588,28 +599,34 @@ static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
     HRESULT hr;
     STATSTG stat;
     ULARGE_INTEGER current_pos;
-    void *bits;
+    void *bits, *hdr;
     METAFILEPICT *mfpict;
     HGLOBAL hmfpict;
-    PresentationDataHeader header;
-    CLIPFORMAT clipformat;
     static const LARGE_INTEGER offset_zero;
-    ULONG read;
-
-    if (cache_entry->load_stream_num == STREAM_NUMBER_CONTENTS)
-    {
-        FIXME( "Unimplemented for CONTENTS stream\n" );
-        return E_FAIL;
-    }
+    ULONG read, hdr_size;
+    /* only standard clipformats */
+    struct {
+        DWORD cf[2];
+        PresentationDataHeader pres;
+    } pres_hdr;
+    struct meta_placeable meta_place_rec;

hr = IStream_Stat( stm, &stat, STATFLAG_NONAME );
     if (FAILED( hr )) return hr;

-    hr = read_clipformat( stm, &clipformat );
-    if (FAILED( hr )) return hr;
+    if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
+    {
+        hdr_size = sizeof(pres_hdr);
+        hdr = &pres_hdr;
+    }
+    else
+    {
+        hdr_size = sizeof(struct meta_placeable);
+        hdr = &meta_place_rec;
+    }

-    hr = IStream_Read( stm, &header, sizeof(header), &read );
-    if (hr != S_OK || read != sizeof(header)) return E_FAIL;
+    hr = IStream_Read( stm, hdr, hdr_size, &read );
+    if (hr != S_OK || read != hdr_size) return E_FAIL;

hr = IStream_Seek( stm, offset_zero, STREAM_SEEK_CUR, &current_pos );
     if (FAILED( hr )) return hr;
@@ -632,10 +649,18 @@ static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )

if (SUCCEEDED( hr ))
     {
-        /* FIXME: get this from the stream */
         mfpict->mm = MM_ANISOTROPIC;
-        mfpict->xExt = header.dwObjectExtentX;
-        mfpict->yExt = header.dwObjectExtentY;
+        /* FIXME: get this from the stream */
+        if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
+        {
+            mfpict->xExt = pres_hdr.pres.dwObjectExtentX;
+            mfpict->yExt = pres_hdr.pres.dwObjectExtentY;
+        }
+        else
+        {
+            mfpict->xExt = (meta_place_rec.bounding_box[2] * 2540) / meta_place_rec.inch;
+            mfpict->yExt = (meta_place_rec.bounding_box[3] * 2540) / meta_place_rec.inch;
+        }
         mfpict->hMF = SetMetaFileBitsEx( stat.cbSize.u.LowPart, bits );
         if (!mfpict->hMF)
             hr = E_FAIL;
@@ -916,18 +941,6 @@ end:
     return hr;
 }

-#include <pshpack2.h>
-struct meta_placeable
-{
-    DWORD key;
-    WORD hwmf;
-    WORD bounding_box[4];
-    WORD inch;
-    DWORD reserved;
-    WORD checksum;
-};
-#include <poppack.h>
-
 static HRESULT save_mfpict(DataCacheEntry *entry, BOOL contents, IStream *stream)
 {
     HRESULT hr = S_OK;

[PATCH 4/4] ole32/tests: Add tests for loading from presentation streams to data cache.

[Sergio Gómez Del Real]

Signed-off-by: Sergio Gómez Del Real 
---
 dlls/ole32/datacache.c  |  10 +--
 dlls/ole32/tests/ole2.c | 159 ++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 144 insertions(+), 25 deletions(-)

diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
index 5a56eb76b5..5bf57035d3 100644
--- a/dlls/ole32/datacache.c
+++ b/dlls/ole32/datacache.c
@@ -1318,7 +1318,7 @@ static inline DWORD tymed_from_cf( DWORD cf )
  * automatic entry is re-assigned a new connection id, and moved to
  * the end of the list.
  */
-static HRESULT create_automatic_entry(DataCache *cache, const CLSID *clsid)
+static HRESULT create_automatic_entry(DataCache *cache, const CLSID *clsid, BOOL entry_was_destroyed)
 {
     static const struct data
     {
@@ -1335,7 +1335,7 @@ static HRESULT create_automatic_entry(DataCache *cache, const CLSID *clsid)
     struct list *head;
     DataCacheEntry *entry;

-    if (IsEqualCLSID( &cache->clsid, clsid )) return S_OK;
+    if (IsEqualCLSID( &cache->clsid, clsid ) && !entry_was_destroyed) return S_OK;

/* move and reassign any pre-existing automatic entry */
     if ((head = list_head( &cache->cache_list )))
@@ -1740,7 +1740,7 @@ static HRESULT WINAPI DataCache_InitNew(
     IStorage_AddRef(This->presentationStorage);
     This->dirty = TRUE;
     ReadClassStg( pStg, &clsid );
-    hr = create_automatic_entry( This, &clsid );
+    hr = create_automatic_entry( This, &clsid, FALSE );
     if (FAILED(hr))
     {
         IStorage_Release( pStg );
@@ -1853,7 +1853,7 @@ static HRESULT WINAPI DataCache_Load( IPersistStorage *iface, IStorage *stg )
         DataCacheEntry_Destroy( This, entry );

ReadClassStg( stg, &clsid );
-    hr = create_automatic_entry( This, &clsid );
+    hr = create_automatic_entry( This, &clsid, TRUE );
     if (FAILED( hr )) return hr;

This->clsid = clsid;
@@ -3013,7 +3013,7 @@ static DataCache* DataCache_Construct(
   newObject->dirty = FALSE;
   newObject->running_object = NULL;

-  create_automatic_entry( newObject, clsid );
+  create_automatic_entry( newObject, clsid, FALSE );
   newObject->clsid = *clsid;

return newObject;
diff --git a/dlls/ole32/tests/ole2.c b/dlls/ole32/tests/ole2.c
index 3bd4acce92..8aad62c1ce 100644
--- a/dlls/ole32/tests/ole2.c
+++ b/dlls/ole32/tests/ole2.c
@@ -4078,6 +4078,69 @@ static void check_storage_contents(IStorage *stg, const struct storage_def *stg_
     }
 }

+static void check_medium_contents(const STGMEDIUM *stgmed_def, STGMEDIUM *stgmed_ld, FORMATETC *fmt)
+{
+    BYTE *data_def, *data_ld;
+    int i;
+    int data_size = 0, data_size_def, data_size_ld;
+
+    if (fmt->cfFormat == CF_METAFILEPICT)
+    {
+        METAFILEPICT *mfpict_def = GlobalLock(U(stgmed_def)->hMetaFilePict);
+        METAFILEPICT *mfpict_ld = GlobalLock(U(stgmed_ld)->hMetaFilePict);
+
+        data_size_def = GetMetaFileBitsEx(mfpict_def->hMF, 0, NULL);
+        data_size_ld = GetMetaFileBitsEx(mfpict_ld->hMF, 0, NULL);
+        ok(data_size_def == data_size_ld, "Sizes differ. Is %d; should be %d\n", data_size_ld, data_size_def);
+        if (data_size_def == data_size_ld)
+        {
+            data_def = HeapAlloc(GetProcessHeap(), 0, data_size_def);
+            data_ld = HeapAlloc(GetProcessHeap(), 0, data_size_ld);
+            GetMetaFileBitsEx(mfpict_def->hMF, data_size_def, data_def);
+            GetMetaFileBitsEx(mfpict_ld->hMF, data_size_ld, data_ld);
+            data_size = data_size_def;
+        }
+    }
+    else if (fmt->cfFormat == CF_ENHMETAFILE)
+    {
+        data_size_def = GetEnhMetaFileBits(stgmed_def->hEnhMetaFile, 0, NULL);
+        data_size_ld = GetEnhMetaFileBits(stgmed_ld->hEnhMetaFile, 0, NULL);
+        ok(data_size_def == data_size_ld, "Sizes differ. Is %d; should be %d\n", data_size_ld, data_size_def);
+        if (data_size_def == data_size_ld)
+        {
+            data_def = HeapAlloc(GetProcessHeap(), 0, data_size_def);
+            data_ld = HeapAlloc(GetProcessHeap(), 0, data_size_ld);
+            GetEnhMetaFileBits(stgmed_def->hEnhMetaFile, data_size_def, data_def);
+            GetEnhMetaFileBits(stgmed_ld->hEnhMetaFile, data_size_ld, data_ld);
+            data_size = data_size_def;
+        }
+    }
+    else if (fmt->cfFormat == CF_DIB)
+    {
+        data_def = GlobalLock(stgmed_def->hGlobal);
+        data_ld = GlobalLock(stgmed_ld->hGlobal);
+
+        data_size = sizeof(dib);
+    }
+
+    for (i = 0; i < data_size; i++)
+    {
+        ok(*data_def == *data_ld, "Data expected and data read does not coincide\n");
+        if (*data_def != *data_ld) break;
+    }
+
+    if (fmt->cfFormat == CF_DIB)
+    {
+        GlobalUnlock(stgmed_def->hGlobal);
+        GlobalUnlock(stgmed_ld->hGlobal);
+    }
+    else if (fmt->cfFormat == CF_METAFILEPICT)
+    {
+        GlobalUnlock(U(stgmed_def)->hMetaFilePict);
+        GlobalUnlock(U(stgmed_ld)->hMetaFilePict);
+    }
+}
+
 static IStorage *create_storage_from_def(const struct storage_def *stg_def)
 {
     HRESULT hr;
@@ -4250,6 +4313,7 @@ static void test_data_cache_save_data(void)
     IStorage *doc;
     IOleCache2 *cache;
     IPersistStorage *persist;
+    IDataObject *odata;
     int enumerated_streams, matched_streams, i;
     DWORD dummy;
     struct tests_data_cache
@@ -4258,6 +4322,7 @@ static void test_data_cache_save_data(void)
         int num_fmts, num_set;
         const CLSID *clsid;
         struct storage_def stg_def;
+        STGMEDIUM stgmed_def[MAX_FMTS];
     };

static struct tests_data_cache *pdata, data[] =
@@ -4274,8 +4339,9 @@ static void test_data_cache_save_data(void)
                 &CLSID_WineTestOld, 4, { { "\2OlePres000", CF_DIB, DVASPECT_CONTENT, 0, NULL, 0 },
                                          { "\2OlePres001", CF_METAFILEPICT, DVASPECT_CONTENT, 0, NULL, 0 },
                                          { "\2OlePres002", CF_ENHMETAFILE, DVASPECT_CONTENT, 0, NULL, 0 },
-                                         { "\2OlePres003", 0, DVASPECT_DOCPRINT, 0, NULL, 0 } }
-            }
+                                         { "\2OlePres003", 0, DVASPECT_DOCPRINT, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         /* without setting data */
         {
@@ -4288,8 +4354,9 @@ static void test_data_cache_save_data(void)
             {
                 &CLSID_WineTestOld, 3, { { "\2OlePres000", CF_DIB, DVASPECT_CONTENT, 0, NULL, 0 },
                                          { "\2OlePres001", CF_METAFILEPICT, DVASPECT_CONTENT, 0, NULL, 0 },
-                                         { "\2OlePres002", CF_ENHMETAFILE, DVASPECT_CONTENT, 0, NULL, 0 } }
-            }
+                                         { "\2OlePres002", CF_ENHMETAFILE, DVASPECT_CONTENT, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         /* static picture clsids */
         {
@@ -4298,8 +4365,9 @@ static void test_data_cache_save_data(void)
             },
             1, 1, &CLSID_Picture_Dib,
             {
-                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } }
-            }
+                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         {
             {
@@ -4307,8 +4375,9 @@ static void test_data_cache_save_data(void)
             },
             1, 1, &CLSID_Picture_Metafile,
             {
-                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } }
-            }
+                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         {
             {
@@ -4316,8 +4385,9 @@ static void test_data_cache_save_data(void)
             },
             1, 1, &CLSID_Picture_EnhMetafile,
             {
-                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } }
-            }
+                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         /* static picture clsids without setting any data */
         {
@@ -4326,8 +4396,9 @@ static void test_data_cache_save_data(void)
             },
             1, 0, &CLSID_Picture_Dib,
             {
-                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } }
-            }
+                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         {
             {
@@ -4335,8 +4406,9 @@ static void test_data_cache_save_data(void)
             },
             1, 0, &CLSID_Picture_Metafile,
             {
-                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } }
-            }
+                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         {
             {
@@ -4344,8 +4416,9 @@ static void test_data_cache_save_data(void)
             },
             1, 0, &CLSID_Picture_EnhMetafile,
             {
-                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } }
-            }
+                &CLSID_WineTestOld, 1, { { "CONTENTS", -1, 0, 0, NULL, 0 } },
+            },
+            { { 0 } }
         },
         {
             {
@@ -4366,9 +4439,9 @@ static void test_data_cache_save_data(void)
             ok(SUCCEEDED(hr), "unexpected %#x\n", hr);
             if (i < pdata->num_set)
             {
-                get_stgmedium(pdata->fmts[i].cfFormat, &stgmed);
-                get_stgdef(&pdata->stg_def, pdata->fmts[i].cfFormat, &stgmed, i);
-                hr = IOleCache2_SetData(cache, &pdata->fmts[i], &stgmed, TRUE);
+                get_stgmedium(pdata->fmts[i].cfFormat, &pdata->stgmed_def[i]);
+                get_stgdef(&pdata->stg_def, pdata->fmts[i].cfFormat, &pdata->stgmed_def[i], i);
+                hr = IOleCache2_SetData(cache, &pdata->fmts[i], &pdata->stgmed_def[i], FALSE);
                 ok(hr == S_OK, "unexpected %#x\n", hr);
             }
         }
@@ -4399,12 +4472,58 @@ static void test_data_cache_save_data(void)
         ok(enumerated_streams == pdata->stg_def.stream_count, "created %d != def streams %d\n",
            enumerated_streams, pdata->stg_def.stream_count);

+        IPersistStorage_Release(persist);
+        IOleCache2_Release(cache);
+
+        /* now test _Load/_GetData using the storage we used for _Save */
+        hr = CreateDataCache(NULL, pdata->clsid, &IID_IOleCache2, (void **)&cache);
+        ok(hr == S_OK, "unexpected %#x\n", hr);
+        hr = IOleCache2_QueryInterface(cache, &IID_IPersistStorage, (void **)&persist);
+        ok(hr == S_OK, "unexpected %#x\n", hr);
+        for (i = 0; i < pdata->num_fmts; i++)
+        {
+            hr = IOleCache2_Cache(cache, &pdata->fmts[i], 0, &dummy);
+            ok(SUCCEEDED(hr), "unexpected %#x\n", hr);
+        }
+
+        if (IsEqualCLSID(pdata->clsid, &CLSID_Picture_Dib))
+        {
+            hr = IStorage_SetClass(doc, &CLSID_Picture_Dib);
+            ok(hr == S_OK, "unexpected %#x\n", hr);
+        }
+        else if (IsEqualCLSID(pdata->clsid, &CLSID_Picture_Metafile))
+        {
+            hr = IStorage_SetClass(doc, &CLSID_Picture_Metafile);
+            ok(hr == S_OK, "unexpected %#x\n", hr);
+        }
+        else if (IsEqualCLSID(pdata->clsid, &CLSID_Picture_EnhMetafile))
+        {
+            hr = IStorage_SetClass(doc, &CLSID_Picture_EnhMetafile);
+            ok(hr == S_OK, "unexpected %#x\n", hr);
+        }
+        trace("IPersistStorage_Load\n");
+        hr = IPersistStorage_Load(persist, doc);
+        ok(hr == S_OK, "unexpected %#x\n", hr);
+
+        hr = IOleCache2_QueryInterface(cache, &IID_IDataObject, (void **)&odata);
+        ok(hr == S_OK, "unexpected %#x\n", hr);
         for (i = 0; i < pdata->num_set; i++)
-            HeapFree(GetProcessHeap(), 0, (void *)pdata->stg_def.stream[i].data);
+        {
+            hr = IDataObject_GetData(odata, &pdata->fmts[i], &stgmed);
+            ok(hr == S_OK, "unexpected %#x\n", hr);

+            check_medium_contents(&pdata->stgmed_def[i], &stgmed, &pdata->fmts[i]);
+            ReleaseStgMedium(&stgmed);
+            ReleaseStgMedium(&pdata->stgmed_def[i]);
+        }
+
+        IDataObject_Release(odata);
         IPersistStorage_Release(persist);
         IStorage_Release(doc);
         IOleCache2_Release(cache);
+        for (i = 0; i < pdata->num_set; i++)
+            HeapFree(GetProcessHeap(), 0, (void *)pdata->stg_def.stream[i].data);
+
     }
 }

[PATCH 4/4] ole32/tests: Add tests for loading from presentation streams to data cache.

[Huw Davies]

On Tue, Mar 27, 2018 at 11:40:01AM -0500, Sergio Gómez Del Real wrote:
> Signed-off-by: Sergio Gómez Del Real
>
> dlls/ole32/datacache.c  |  10 +--
>  dlls/ole32/tests/ole2.c | 159 ++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 144 insertions(+), 25 deletions(-)

Whoa!  Your commit message mentions tests, but it doesn't mention
changing the implementation.  That needs at least to be explained
and probably moved a separate patch...

[Huw Davies]

On Tue, Mar 27, 2018 at 11:39:58AM -0500, Sergio Gómez Del Real wrote:
> +static HRESULT load_emf( DataCacheEntry *cache_entry, IStream *stm )
> +{
> -
> - if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
> - {
> -
>    HMETAFILEPICT hmfpict;
> -
>    METAFILEPICT *mfpict;
> -
> -
>    load_mf_pict(cache_entry, stm);
> -
>    hmfpict = cache_entry->stgmedium.u.hMetaFilePict;
> -
>    synthesize_emf(hmfpict, &cache_entry->stgmedium);
> -
>    mfpict = GlobalLock(hmfpict);
> -
>    DeleteMetaFile(mfpict->hMF);
> -
>    GlobalFree(hmfpict);

There's no error checking in this block at all.  Also the clean way to
do this would be declare a STGMEDIUM temporary variable and pass the
address of that as the 2nd param of synthesize_emf().  Then call
ReleaseStgMedium() on the cache_entry's stgmedium and copy the
temporary stgmedium to the cache_entry.
> -
>
> }
> -
>
> else
> -
>
> {
> -
>    HRESULT hr;
> -
>    STATSTG stat;
> -
>    void *data;
> -
>    DWORD size_bits;
> -
>    ULONG read;
> -
> -
>    hr = IStream_Stat( stm, &stat, STATFLAG_NONAME );
> -
>    if (FAILED( hr )) return hr;
>
> -
>    data = HeapAlloc( GetProcessHeap(), 0, stat.cbSize.u.LowPart );
> -
>    if (!data) return E_OUTOFMEMORY;
> -
> -
>    hr = IStream_Read( stm, data, stat.cbSize.u.LowPart, &read );
> -
>    if (hr != S_OK || read != stat.cbSize.u.LowPart) hr = E_FAIL;
> -

Just setting hr to E_FAIL isn't enough here, right?
> -
>    size_bits = read - sizeof(DWORD) - sizeof(ENHMETAHEADER);
> -
>    if (size_bits <= 0)
> -
>    {
> -
>        HeapFree( GetProcessHeap(), 0, data );
> -
>        return E_FAIL;
> -
>    }
> -
>    cache_entry->stgmedium.u.hEnhMetaFile = SetEnhMetaFileBits( size_bits, (BYTE *)data + (read - size_bits) );
> -
>    cache_entry->stgmedium.tymed = TYMED_ENHMF;
> -
>    cache_entry->stgmedium.pUnkForRelease = NULL;

You leak data here.
> - }
> -
> - return S_OK;
>
> }
>
> /************************************************************************
> @@ -736,6 +805,10 @@ static HRESULT DataCacheEntry_LoadData(DataCacheEntry *cache_entry, IStorage *st
>          hr = load_dib( cache_entry, stm );
>          break;
> - case CF_ENHMETAFILE:
> -
>    hr = load_emf( cache_entry, stm );
> -
>    break;
> - default:
>        FIXME( "Unimplemented clip format %x\n", cache_entry->fmtetc.cfFormat );
>        hr = E_NOTIMPL;
>
> @@ -1118,30 +1191,6 @@ static HRESULT synthesize_bitmap( HGLOBAL dib, STGMEDIUM *med )
>      return hr;
>  }
>
> -static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
> -{
> - METAFILEPICT *pict;
> - HRESULT hr = E_FAIL;
> - UINT size;
> - void *bits;
> -
> - if (!(pict = GlobalLock( data ))) return hr;
> -
> - size = GetMetaFileBitsEx( pict->hMF, 0, NULL );
> - if ((bits = HeapAlloc( GetProcessHeap(), 0, size )))
> - {
> -
>    GetMetaFileBitsEx( pict->hMF, size, bits );
> -
>    med->u.hEnhMetaFile = SetWinMetaFileBits( size, bits, NULL, pict );
> -
>    HeapFree( GetProcessHeap(), 0, bits );
> -
>    med->tymed = TYMED_ENHMF;
> -
>    med->pUnkForRelease = NULL;
> -
>    hr = S_OK;
> - }
> -
> - GlobalUnlock( data );
> - return hr;
>
> -}
> -
>
> static HRESULT DataCacheEntry_SetData(DataCacheEntry *cache_entry,
>                                        const FORMATETC *formatetc,
>                                        STGMEDIUM *stgmedium,
> -- 
> 2.14.1