Thanks for your reply, I will fix the places you mentioned to clean
things up correctly. But other places in olepicture should also be fixed
in this case. I will check that.
Max
On Mon, 2005-02-28 at 15:22 -0600, Robert Shearman wrote:
> 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
>
