Re: OLEAUT32: Use the FAILED macro to check for failure of interface methods.

[Robert Shearman]

Mike McCormack wrote:
> }
>   hr=IStream_Read(pStm,header,8,&xread);
> - if (hr || xread!=8) {
> - if (FAILED(hr) || xread!=8) {
>    FIXME("Failure while reading picture header (hr is %lx, nread is %ld).\n",hr,xread);
>    return hr;
> }
>
> @@ -1128,10 +1128,10 @@ static HRESULT WINAPI OLEPictureImpl_Loa
>           while (xread < origsize) {
>               hr = IStream_Read(pStm,xbuf+xread,origsize-xread,&nread);
>               xread+=nread;
> -
>          if (hr || !nread)
> -
>          if (FAILED(hr) || !nread)
>             break;
>     }
> -
>      if (!nread || hr) /* done, or error */
> -
>      if (!nread || FAILED(hr)) /* done, or error */
>         break;
>     if (xread == origsize) {
>         origsize += sizeinc;
>
> @@ -1139,7 +1139,7 @@ static HRESULT WINAPI OLEPictureImpl_Loa
>               xbuf = HeapReAlloc (GetProcessHeap(), HEAP_ZERO_MEMORY, xbuf, origsize);
>           }
>       }
> -
>  if (hr)
> -
>  if (FAILED(hr))
>     TRACE("hr in no-stat loader case is %08lx\n", hr);
> TRACE("loaded %ld bytes.\n", xread);
> This->datalen = xread;
>
> @@ -1155,7 +1155,7 @@ static HRESULT WINAPI OLEPictureImpl_Loa
>           ULONG nread;
>           hr = IStream_Read(pStm,xbuf+xread,This->datalen-xread,&nread);
>           xread+=nread;
> -
>      if (hr || !nread)
> -
>      if (FAILED(hr) || !nread)
>         break;
> }
> if (xread != This->datalen)

These changes are wrong. IStream_Read can return S_FALSE, which means 
that the call was successful, but didn't do what was intended. From MSDN:

S_FALSE
    The data cannot be read from the stream object. Depending on the
    implementation, either S_FALSE or an error code could be returned
    when reading past the end of the stream.

OLEAUT32: Use the FAILED macro to check for failure of interface methods.

[Mike McCormack]

Robert Shearman wrote:
> These changes are wrong. IStream_Read can return S_FALSE, which means 
> that the call was successful, but didn't do what was intended. From MSDN:
>
> S_FALSE
>    The data cannot be read from the stream object. Depending on the
>    implementation, either S_FALSE or an error code could be returned
>    when reading past the end of the stream.

Fair enough.   Then perhaps the ole storage code is broken, because it 
returns S_FALSE when it correctly read the requested amount of data, and 
IOlePicture_Load was failing to read a bitmap from an MSI package.

Mike

OLEAUT32: Use the FAILED macro to check for failure of interface methods.

[Marcus Meissner]

On Sat, Dec 31, 2005 at 11:12:33AM +0900, Mike McCormack wrote:
> Robert Shearman wrote:
> > These changes are wrong. IStream_Read can return S_FALSE, which means 
> > that the call was successful, but didn't do what was intended. From MSDN:
> >
> > S_FALSE
> >   The data cannot be read from the stream object. Depending on the
> >   implementation, either S_FALSE or an error code could be returned
> >   when reading past the end of the stream.
>
> Fair enough.   Then perhaps the ole storage code is broken, because it 
> returns S_FALSE when it correctly read the requested amount of data, and 
> IOlePicture_Load was failing to read a bitmap from an MSI package.

Do you have a testcase installer?

Ciao, Marcus

OLEAUT32: Use the FAILED macro to check for failure of interface methods.

[Robert Shearman]

Marcus Meissner wrote:
> On Sat, Dec 31, 2005 at 11:12:33AM +0900, Mike McCormack wrote:
> > Robert Shearman wrote:
> > > These changes are wrong. IStream_Read can return S_FALSE, which means 
> > > that the call was successful, but didn't do what was intended. From MSDN:
> > >
> > > S_FALSE
> > >  The data cannot be read from the stream object. Depending on the
> > >  implementation, either S_FALSE or an error code could be returned
> > >  when reading past the end of the stream.
> >
> > Fair enough.   Then perhaps the ole storage code is broken, because it 
> > returns S_FALSE when it correctly read the requested amount of data, and 
> > IOlePicture_Load was failing to read a bitmap from an MSI package.
>
> Do you have a testcase installer?

No need. Working with Mike, I've found the bug and I'll send a patch soon.