Re: Implements OleLoadPicturePath

[Robert Shearman]

Maxime Bellengé wrote:
> Hello, here is an implementation of OleLoadPicturePath.
> The filename can be either a local file or an url.
> In case of a url, I needed a user-agent for wininet. I put a dummy value
> as I didn't find a default wine value. Is there a default value
> somewhere for the whole project ?
>
> a+
>
> Max
>
> ChangeLog:
> - Implements OleLoadPicturePath
> Index: wine/dlls/oleaut32/olepicture.c
>
> RCS file: /home/wine/wine/dlls/oleaut32/olepicture.c,v
> retrieving revision 1.55
> diff -u -r1.55 olepicture.c
> --- wine/dlls/oleaut32/olepicture.c	25 Feb 2005 14:07:57 -0000	1.55
> +++ wine/dlls/oleaut32/olepicture.c	27 Feb 2005 10:51:43 -0000
> @@ -71,7 +71,9 @@
> #include "olectl.h"
> #include "oleauto.h"
> #include "connpt.h"
> +#include "wininet.h"
> #include "wine/debug.h"
> +#include "wine/unicode.h"
>
> #include "wine/wingdi16.h"
> #include "cursoricon.h"
> @@ -1998,11 +2000,122 @@
>    	DWORD dwReserved, OLE_COLOR clrReserved, REFIID riid,
>    	LPVOID *ppvRet )
> {
> - FIXME("(%s,%p,%ld,%08lx,%s,%p): stub\n",
> - static const WCHAR file[] = { 'f','i','l','e',':','/','/',0 };
> - static const WCHAR name[] = { 'w','i','n','e','-','a','g','e','n','t',0};
> - IPicture *ipicture;
> - HANDLE hFile;
> - DWORD dwFileSize;
> - HGLOBAL hGlobal = NULL;
> - DWORD dwBytesRead = 0;
> - IStream *stream;
> - BOOL bRead;
> - IPersistStream *pStream;
> - HRESULT hRes;
> -
> - TRACE("(%s,%p,%ld,%08lx,%s,%p): stub\n",
>      debugstr_w(szURLorPath), punkCaller, dwReserved, clrReserved,
>      debugstr_guid(riid), ppvRet);
> - return E_NOTIMPL;
> - if (!ppvRet) return E_POINTER;
> -
> - if (strncmpW(szURLorPath, file, 7) == 0) {
> -
>  szURLorPath += 7;
> -
> -
>  hFile = CreateFileW(szURLorPath, GENERIC_READ, 0, NULL, OPEN_EXISTING,
> -
> 		   0, NULL);
> -
>  if (hFile == INVALID_HANDLE_VALUE)
> -
>  return E_UNEXPECTED;
> -
> -
>  dwFileSize = GetFileSize(hFile, NULL);
> -
>  if (dwFileSize != INVALID_FILE_SIZE )
> -
>  {
> -
>  hGlobal = GlobalAlloc(GMEM_FIXED,dwFileSize);
> -
>  if ( hGlobal)
> -
>  {
> -
>      bRead = ReadFile(hFile, hGlobal, dwFileSize, &dwBytesRead, NULL);
> -
>      if (!bRead)
> -
>      {
> -
>   GlobalFree(hGlobal);
> -
>   hGlobal = 0;
> -
>      }
> -
>  }
> -
>  }
> -
>  CloseHandle(hFile);      
> - } else {
> -
>  HINTERNET hInternet;
> -
>  BOOL res = FALSE;
> -
> -
>  hInternet = InternetOpenW(name, INTERNET_OPEN_TYPE_PRECONFIG, NULL,
> -
> 		NULL, 0);
> -
>  if (hInternet) 
> -
>  {
> -
>  HINTERNET hFile;
> -
>  hFile = InternetOpenUrlW(hInternet, szURLorPath, NULL, -1L, 
> -
> 	  INTERNET_FLAG_RESYNCHRONIZE, (DWORD_PTR)NULL);
> -
>  if (hFile)
> -
>  {	      
> -
>      DWORD total = 4096;
> -
>      DWORD nbRead;	      
> -
> -
>      hGlobal = GlobalAlloc(GMEM_FIXED, 4096);
> -
>      if (hGlobal)
> -
>      {
> -
>   do {
> -
>       res = InternetReadFile(hFile, hGlobal, 4096, &nbRead);
> -
>       if (res && (nbRead>0))
> -
>       {
> -
> 	  HGLOBAL hNewGlobal;
> -
> 	  hNewGlobal = GlobalReAlloc(hGlobal, total+4096, 0);
> -
> 	  if (!hNewGlobal)
> -
> 	  {
> -
> 	      res = FALSE;
> -
> 	      break;
> -
> 	  }		      
> -
>       }
> -
>   } while (res && (nbRead>0));
> -
>      }
> -
>  }
> -
> -
>  InternetCloseHandle(hInternet);
> -
>  }
> -
>  if (!res) 
> -
>  {
> -
>  GlobalFree(hGlobal);	  
> -
>  }
> - }
> -
> - if (!hGlobal)
> -
>  return E_UNEXPECTED;
> -
> - hRes = CreateStreamOnHGlobal(hGlobal, TRUE, &stream);
> - if (FAILED(hRes))
> - {
> -
>  if (stream != NULL) IStream_Release(stream);

A valid stream will not be returned on failure, so no need for the extra 
check here.
> -
>  GlobalFree(hGlobal);
> -
>  return hRes;
> - }
> -
> - hRes = CoCreateInstance(&CLSID_StdPicture, punkCaller, CLSCTX_INPROC_SERVER,
> -
>    &IID_IPicture, (LPVOID*)&ipicture);
> - if (hRes != S_OK) {

Lack of cleanup code here.
> -
>   return hRes;
> - }
> -
> - hRes = IPicture_QueryInterface(ipicture, &IID_IPersistStream, (LPVOID*)&pStream);
> - if (hRes) {
> -
>  IPicture_Release(ipicture);

And here.
> -
>  return hRes;
> - }
> -
> - IPersistStream_Load(pStream, stream);

You should probably check the return value from this function as it is 
kinda vital to the function you are implementing.
> - IPersistStream_Release(pStream);
> - IStream_Release(stream);
> - hRes = IPicture_QueryInterface(ipicture,riid,ppvRet);
> - if (hRes)
> -
>  FIXME("Failed to get interface %s from IPicture.\n",debugstr_guid(riid));
> -
> - IPicture_Release(ipicture);
> - return hRes;
>
> }
>
> /*******************************************************************************
> Index: wine/dlls/oleaut32/Makefile.in
>
> RCS file: /home/wine/wine/dlls/oleaut32/Makefile.in,v
> retrieving revision 1.57
> diff -u -r1.57 Makefile.in
> --- wine/dlls/oleaut32/Makefile.in	27 Oct 2004 00:47:53 -0000	1.57
> +++ wine/dlls/oleaut32/Makefile.in	27 Feb 2005 10:52:08 -0000
> @@ -4,7 +4,7 @@
> SRCDIR    = @srcdir@
> VPATH     = @srcdir@
> MODULE    = oleaut32.dll
> -IMPORTS   = ole32 rpcrt4 user32 gdi32 advapi32 kernel32 ntdll
> +IMPORTS   = ole32 rpcrt4 user32 gdi32 advapi32 kernel32 ntdll wininet
> DELAYIMPORTS = comctl32
> EXTRALIBS = $(LIBUNICODE) -luuid

I don't think oleaut32 should be importing from wininet for just the 
implementation of OleLoadPicturePath. It surprises me that there is not 
already an implementation of IStream for URLs that can be re-used. If 
there is no alternative, I think we should just make wininet a delayed 
import.

Rob