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.



-- 
Rob Shearman

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.

-- 
Rob Shearman