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

[Huw Davies]

On Fri, Mar 30, 2018 at 12:11:34PM -0500, Sergio Gómez Del Real wrote:
> Signed-off-by: Sergio Gómez Del Real
>
> dlls/ole32/datacache.c | 49 +++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 12 deletions(-)
>
> diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
> index 799e14a867..e6cda331dd 100644
> --- a/dlls/ole32/datacache.c
> +++ b/dlls/ole32/datacache.c
> @@ -610,6 +610,20 @@ static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
>      return hr;
>  }
>
> +static HRESULT read_pres_header( IStream *stm, DWORD *clip_format,
> -
>                             PresentationDataHeader *pres_hdr )
>
> +{
> - ULONG read;
> - HRESULT hr;
> -
> - hr = IStream_Read( stm, clip_format, sizeof(DWORD) * 2, &read );
> - if (hr != S_OK || read != sizeof(DWORD) * 2) return E_FAIL;

Note, we already have read_clipformat() which correctly handles
the variable length stuff.  This should either call that or we
could have this function's callers do it themselves.  The latter
may be simpler, in which case this function ceases to be useful,
as you may as well do the IStream_Read in this function's callers.
> - hr = IStream_Read( stm, pres_hdr, sizeof(PresentationDataHeader), &read );
> - if (hr != S_OK || read != sizeof(PresentationDataHeader)) return E_FAIL;
> -
> - return S_OK;
>
> +}
> -
>
> static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
>  {
>      HRESULT hr;
> @@ -689,32 +703,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;

This structure isn't very useful.  All you need a CLIPFORMAT and a
PresentationDataHeader as two separate variables.
> -
>
> 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, &pres_hdr.pres );
> -
>    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;
>
> @@ -727,7 +751,8 @@ 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 && file.bfOffBits)
> {
>     LARGE_INTEGER skip;
>
> -- 
> 2.14.1