Re: [PATCH] oleaut32: OLEPictureImpl_Load uses Seek(STREAM_SEEK_END) to find the stream length

[Marcus Meissner]

On Thu, Sep 15, 2011 at 02:09:56PM +0200, Bernhard Loos wrote:
> ---
>  dlls/oleaut32/olepicture.c       |  148 +++++++++++++-------------------------
>  dlls/oleaut32/tests/olepicture.c |   16 ++---
>  2 files changed, 54 insertions(+), 110 deletions(-)

Did make check work for you even? Marvin does not like it at least.

This will break NonStatable streams, also seekability of streams can
not be assumed.

Ciao, Marcus
> diff --git a/dlls/oleaut32/olepicture.c b/dlls/oleaut32/olepicture.c
> index 8081785..725207f 100644
> --- a/dlls/oleaut32/olepicture.c
> +++ b/dlls/oleaut32/olepicture.c
> @@ -1324,15 +1324,13 @@ static HRESULT OLEPictureImpl_LoadAPM(OLEPictureImpl *This,
>   */
>  static HRESULT WINAPI OLEPictureImpl_Load(IPersistStream* iface, IStream *pStm) {
>    HRESULT	hr;
> -  BOOL		headerisdata;
> -  BOOL		statfailed = FALSE;
> -  ULONG		xread, toread;
> -  ULONG 	headerread;
> +  ULONG		xread, toread = 0;
>    BYTE 		*xbuf;
>    DWORD		header[2];
>    WORD		magic;
> -  STATSTG       statstg;
>    OLEPictureImpl *This = impl_from_IPersistStream(iface);
> +  ULARGE_INTEGER end, start;
> +  LARGE_INTEGER offset;
>
> TRACE("(%p,%p)\n",This,pStm);
>
> @@ -1348,104 +1346,56 @@ static HRESULT WINAPI OLEPictureImpl_Load(IPersistStream* iface, IStream *pStm)
>     * At least in Visual Basic 6, resource streams, valid headers are
>     *    header[0] == "lt\0\0",
>     *    header[1] == length_of_stream.
> -   *
> -   * Also handle streams where we do not have a working "Stat" method by
> -   * reading all data until the end of the stream.
>     */
> -  hr = IStream_Stat(pStm,&statstg,STATFLAG_NONAME);
> -  if (hr != S_OK) {
> -      TRACE("stat failed with hres %x, proceeding to read all data.\n",hr);
> -      statfailed = TRUE;
> -      /* we will read at least 8 byte ... just right below */
> -      statstg.cbSize.QuadPart = 8;
> -  }
> +  offset.QuadPart = 0;
>
> -  toread = 0;
> -  headerread = 0;
> -  headerisdata = FALSE;
>    do {
> -      hr = IStream_Read(pStm, header, 8, &xread);
> -      if (hr != S_OK || xread!=8) {
> -          ERR("Failure while reading picture header (hr is %x, nread is %d).\n",hr,xread);
> -          return (hr?hr:E_FAIL);
> -      }
> -      headerread += xread;
> -      xread = 0;
> -      
> -      if (!memcmp(&(header[0]),"lt\0\0", 4) && (statfailed || (header[1] + headerread <= statstg.cbSize.QuadPart))) {
> -          if (toread != 0 && toread != header[1]) 
> -              FIXME("varying lengths of image data (prev=%u curr=%u), only last one will be used\n",
> -                  toread, header[1]);
> -          toread = header[1];
> -          if (toread == 0) break;
> -      } else {
> -          if (!memcmp(&(header[0]), "GIF8",     4) ||   /* GIF header */
> -              !memcmp(&(header[0]), "BM",       2) ||   /* BMP header */
> -              !memcmp(&(header[0]), "\xff\xd8", 2) ||   /* JPEG header */
> -              (header[0] == EMR_HEADER)            ||   /* EMF header */
> -              (header[1] > statstg.cbSize.QuadPart)||   /* invalid size */
> -              (header[1]==0)
> -          ) {/* Found start of bitmap data */
> -              headerisdata = TRUE;
> -              if (toread == 0) 
> -              	  toread = statstg.cbSize.QuadPart-8;
> -              else toread -= 8;
> -              xread = 8;
> -          } else {
> -              FIXME("Unknown stream header magic: %08x\n", header[0]);
> -              toread = header[1];
> -          }
> -      }
> -  } while (!headerisdata);
> -
> -  if (statfailed) { /* we don't know the size ... read all we get */
> -      int sizeinc = 4096;
> -      int origsize = sizeinc;
> -      ULONG nread = 42;
> -
> -      TRACE("Reading all data from stream.\n");
> -      xbuf = HeapAlloc (GetProcessHeap(), HEAP_ZERO_MEMORY, origsize);
> -      if (headerisdata)
> -          memcpy (xbuf, header, 8);
> -      while (1) {
> -          while (xread < origsize) {
> -              hr = IStream_Read(pStm,xbuf+xread,origsize-xread,&nread);
> -              xread += nread;
> -              if (hr != S_OK || !nread)
> -                  break;
> -          }
> -          if (!nread || hr != S_OK) /* done, or error */
> -              break;
> -          if (xread == origsize) {
> -              origsize += sizeinc;
> -              sizeinc = 2*sizeinc; /* exponential increase */
> -              xbuf = HeapReAlloc (GetProcessHeap(), HEAP_ZERO_MEMORY, xbuf, origsize);
> -          }
> -      }
> -      if (hr != S_OK)
> -          TRACE("hr in no-stat loader case is %08x\n", hr);
> -      TRACE("loaded %d bytes.\n", xread);
> -      This->datalen = xread;
> -      This->data    = xbuf;
> -  } else {
> -      This->datalen = toread+(headerisdata?8:0);
> -      xbuf = This->data = HeapAlloc (GetProcessHeap(), HEAP_ZERO_MEMORY, This->datalen);
> -      if (!xbuf)
> -          return E_OUTOFMEMORY;
> -
> -      if (headerisdata)
> -          memcpy (xbuf, header, 8);
> -
> -      while (xread < This->datalen) {
> -          ULONG nread;
> -          hr = IStream_Read(pStm,xbuf+xread,This->datalen-xread,&nread);
> -          xread += nread;
> -          if (hr != S_OK || !nread)
> -              break;
> -      }
> -      if (xread != This->datalen)
> -          ERR("Could only read %d of %d bytes out of stream?\n",xread,This->datalen);
> +    hr = IStream_Seek(pStm, offset, STREAM_SEEK_CUR, &start);
> +    if (FAILED(hr))
> +        return hr;
> +
> +    hr = IStream_Read(pStm, header, 8, &xread);
> +    if (FAILED(hr) || xread!=8) {
> +        ERR("Failure while reading picture header (hr is %x, nread is %d).\n",hr,xread);
> +        return (FAILED(hr)?hr:E_FAIL);
> +    }
> +
> +    if (memcmp(&(header[0]),"lt", 2)) {
> +        if (!toread) {
> +            hr = IStream_Seek(pStm, offset, STREAM_SEEK_END, &end);
> +            if (FAILED(hr))
> +                return hr;
> +            toread = end.QuadPart - start.QuadPart;
> +        }
> +        offset.QuadPart = start.QuadPart;
> +        hr = IStream_Seek(pStm, offset, STREAM_SEEK_SET, NULL);
> +        if (FAILED(hr))
> +            return hr;
> +        break;
> +    }
> +    else {
> +        toread = header[1];
> +        if (toread == 0)
> +            break;
> +    }
> +  } while (1);
> +
> +  This->datalen = toread;
> +  xbuf = This->data = HeapAlloc (GetProcessHeap(), HEAP_ZERO_MEMORY, This->datalen);
> +  if (!xbuf)
> +      return E_OUTOFMEMORY;
> +
> +  xread = 0;
> +  while (xread < This->datalen) {
> +      ULONG nread;
> +      hr = IStream_Read(pStm,xbuf+xread,This->datalen-xread,&nread);
> +      xread += nread;
> +      if (FAILED(hr) || !nread)
> +          break;
>    }
> +  if (xread != This->datalen)
> +      ERR("Could only read %d of %d bytes out of stream?\n",xread,This->datalen);
> +
>    if (This->datalen == 0) { /* Marks the "NONE" picture */
>        This->desc.picType = PICTYPE_NONE;
>        return S_OK;
> diff --git a/dlls/oleaut32/tests/olepicture.c b/dlls/oleaut32/tests/olepicture.c
> index 54de1cb..b1fad94 100644
> --- a/dlls/oleaut32/tests/olepicture.c
> +++ b/dlls/oleaut32/tests/olepicture.c
> @@ -1063,20 +1063,14 @@ static HRESULT WINAPI NoStatStreamImpl_Read(
>  {
>    NoStatStreamImpl* const This = impl_from_IStream(iface);
>    void* supportBuffer;
> -  ULONG bytesReadBuffer;
> -  ULONG bytesToReadFromBuffer;
>
> -  if (pcbRead==0)
> -    pcbRead = &bytesReadBuffer;
> -  bytesToReadFromBuffer = min( This->streamSize.u.LowPart - This->currentPosition.u.LowPart, cb);
>    supportBuffer = GlobalLock(This->supportHandle);
> -  memcpy(pv, (char *) supportBuffer+This->currentPosition.u.LowPart, bytesToReadFromBuffer);
> -  This->currentPosition.u.LowPart+=bytesToReadFromBuffer;
> -  *pcbRead = bytesToReadFromBuffer;
> +  memcpy(pv, (char *) supportBuffer+This->currentPosition.u.LowPart, cb);
> +  This->currentPosition.u.LowPart+=cb;
>    GlobalUnlock(This->supportHandle);
> -  if(*pcbRead == cb)
> -    return S_OK;
> -  return S_FALSE;
> +  if(pcbRead)
> +     *pcbRead = cb;
> +  return S_OK;
>  }
>
> static HRESULT WINAPI NoStatStreamImpl_Write(
>

-- 
Working, but not speaking, for the following german company:
SUSE LINUX Products GmbH, HRB 16746 (AG Nuernberg)
Geschaeftsfuehrer: Jeff Hawn, Jennifer Guild, Felix Imendoerffer