Re: implement OleLoadPictureFile
Hello Jeremy, Juan and rest of wine-devel [sorry for shifting quotes around, this makes following my points easier and in this case shouldn't forge you to seem to have said something you didn't say] And now that I know that, I certainly won't be doing it again. Thank you for your understaning. Another Thank you from here. If I provided a patch to dlls/oleaut32/tests/olepicture.c adding tests that verify the behaviors of the functions, would that be accepted? In general, writing tests is okay, as they are an exercise in black-box reverse engineering, which is what's allowed. So yeah, tests would be great. Would it be acceptable for me to write up a description of what the function needs to do, so that someone else can do a clean-room implementation? I think that would be acceptable, yes. We have implemented things with hints from people who've studied disassembly before. Do you have a bug open? Sorry, I've forgotten. If not, please do open one. You can describe your findings there. The best way to do this is (in my oppinion) submitting the testcase again, but without the implementation, and marking the test todo_wine. A bug might be useful, but wouldn't a mail that contains the testcase as patch and the description in the comment part to wine-patches suffice? Be sure to write your findings like (this is hypothetical, as I did not look at your patches and the OlePicture stuff till now): Loads an image from a File. This is just like OleLoadPictureStream, but with a file name instead of a stream as source specification and not like To implement the function, you must create a stream from the file (use the standard OLE file stream object for that), pass the stream to OleLoadPictureStream and finally release the stream. In the error case you should do foo. The second form *is* just publishing what you saw in disassembly, so everyone who reads this mail carefully is in my oppinion everyone reading it carefully is in the same situation as you and shouldn't implement the function. Testcases on the other hand are fine. They just describe *what* to do, but not *how*. Especially for the error case testcases might be interesting. Is there anyone out there who would volunteer to do it if I were to write a description of the function? I would do so, if its just some minutes. Regards, Michael Karcher
Re: ComputeSphereVisibility: a patch
2008/11/17 paulo lesgaz [EMAIL PROTECTED]: The problem is that the test takes the value 0x3f once (if the test is correct), and that my function can not never obtain such a value. Looks like you need to set one of the D3DVIS_INSIDE_FRUSTUM/D3DVIS_INTERSECT_FRUSTUM/D3DVIS_OUTSIDE_FRUSTUM flags, depending on which applies. The values of the flags also suggest INSIDE/OUTSIDE applies to the sphere centers rather than the sphere as a whole. Ie, you can have both D3DVIS_INSIDE_LEFT and D3DVIS_INTERSECT_LEFT set. You would need to write tests for this, of course. +vec.u1.x = m._14 + m._11; +vec.u2.y = m._24 + m._21; +vec.u3.z = m._34 + m._31; +origin_plane = m._44 + m._41; Note that the planes you get from this aren't necessarily normalized.
Re: ntoskrnl.exe: Implement MmGetSystemRoutineAddress
Alistair Leslie-Hughes [EMAIL PROTECTED] wrote:
+if(RtlUnicodeStringToAnsiString(routineNameA, SystemRoutineName, TRUE)
== STATUS_SUCCESS)
+{
+/* We only support functions exported from ntoskrnl.exe or hal.dll */
+hMod = GetModuleHandleW(ntoskrnlW);
+if(hMod)
+{
+ pFunc = GetProcAddress(hMod, routineNameA.Buffer);
+}
You are calling GetModuleHandleW from inside of ntoskrnl.exe, how that
could fail?
+if(!pFunc)
+{
+ hMod = GetModuleHandleW(halW);
+ if(hMod)
+ {
+ pFunc = GetProcAddress(hMod, routineNameA.Buffer);
+ }
+}
+
+RtlFreeAnsiString(routineNameA);
+}
+
+TRACE(Function addresss %p\n, pFunc);
+
+return pFunc;
+}
Please follow the existing coding style, place a space after 'if' and
avoid not necessary braces for single line statements after 'if'.
--
Dmitry.
New error: cp: X and X are the same file
I'm seeing a lot of errors like this lately: cp: `/home/dank/.local/share/icons/3550_shell32.0.xpm' and `/home/dank/.local/share/icons/3550_shell32.0.xpm' are the same file Did the recent rewrite of menu item creation break something?
Re: Race in thread shutdown in imm32?
2008/11/17 James McKenzie [EMAIL PROTECTED]: Rob Shearman wrote: 2008/11/14 Dan Kegel [EMAIL PROTECTED]: I'm seeing the following valgrind warning in three out of eight runs under heavy load: InvalidRead IMM_FreeThreadData:233 DllMain:382 __wine_spec_dll_entry:40 MODULE_InitDLL:911 LdrShutdownThread:2174 call_thread_func:403 start_thread:444 It kind of feels like a race between thread shutdown and process shutdown. Does that ring a bell with anyone? IMM_FreeThreadData can crash if TlsGetValue returns a NULL pointer. On first glance, it doesn't appear possible as IMM_InitThreadData is called for every thread and on process startup. However, as you surmise, it is possible in Wine for a thread to exit after the main process has shut down (i.e. TlsFree(tlsIndex) has been called) and the TLS area has been cleared, causing TlsGetValue to return 0. I believe that Windows terminates all threads on process exit, which would solve this problem. However, the issue could trivially worked around by introducing a NULL pointer check in IMM_FreeThreadData. It would also be a good idea to set tlsIndex to TLS_OUT_OF_INDEXES after TlsFree is called to avoid the possibility of using an un-allocated TLS index. The question is how does Windows implement this feature? Does it throw a 'kill' for all threads and then follows with a 'kill' for the entire process which may hang if one of the threads will not die? If this is the method, we should duplicate it, but do a better job of handing the 'threads that will not die', IMHO. There is no such thing as a thread that will not die on Windows. Windows probably does the equivalent of TerminateThread for each thread on ExitProcess being called, which terminates the thread with no notifications and not cleaning anything up, including the stack AFAIK, so it shouldn't ever fail. I'll look at the MSDN page that Ge van Geldorp pointed out to see if this is discussed. -- Rob Shearman
RE: [1/5] WineD3D: Get some vertex pipeline infrastructure in place
2008/11/17 Stefan Dösinger [EMAIL PROTECTED]:
-} else if (fixup ||
+} else if (device-vertex_pipe-can_convert_d3dcolor || fixup ||
Shouldn't this be redundant if you already handle color conversion in
IWineD3DVertexBufferImpl_FindDecl()?
Not all vertex data comes from a vertex buffer. 'fixup' is always FALSE for
Draw[Indexed]PrimitiveUP and DrawPrimitiveStrided.
-} else {
+} else if(!device-vertex_pipe-can_convert_d3dcolor) {
I don't think this is how we want to do this. I think it makes more
sense to ask the pipeline if it can handle a particular type for a
particular usage. You can handle FLOAT16 support in the same way then.
Good point, although in practise we don't have to be able to handle all types
with all usages. On some windows drivers e.g. FLOAT16 colors and fixed function
fail, and ati returns an error. I think we can make use of that to keep the
code simpler.
Also note that EXT_vertex_array_bgra adds support for D3DCOLOR, but
only for diffuse, specular and generic attributes. That should be
enough for anything we come across, but with a can_convert_d3dcolor
flag like that you can't handle that extension properly.
Hmm, I think I should make this a function instead of a flag to deal with
EXT_vertex_array_bgra
So better don't apply this patch for now
Re: msxml3: Determine whether to call xsltInit() or not at run time.
Am Montag, den 17.11.2008, 15:09 +0100 schrieb Francois Gouget: configure will need to be regenerated. I did not include it here since the consensus seems to be that it should not be included. Just a nitpick: Same applies for config.h.in, then. As a reminder, the issue is that it's not because the libxslt.so we compile against has xlstInit() that the one we load at runtime will have it too. And vice versa. Wait a moment. I don't know about Wine's dl* functions, but can't we use dlsym with the RTLD_DEFAULT handle to look in the libxslt msxml3.dll.so is linked with anyway? That would do away with the need to know the soname during compilation. As it is hardcoded into the executable during link, hard-coding the soname doesn't hurt that much too, but still it gets simpler Regards, Michael Karcher
Re: [1/5] WineD3D: Get some vertex pipeline infrastructure in place
2008/11/17 Stefan Dösinger [EMAIL PROTECTED]: 2008/11/17 Stefan Dösinger [EMAIL PROTECTED]: -} else if (fixup || +} else if (device-vertex_pipe-can_convert_d3dcolor || fixup || Shouldn't this be redundant if you already handle color conversion in IWineD3DVertexBufferImpl_FindDecl()? Not all vertex data comes from a vertex buffer. 'fixup' is always FALSE for Draw[Indexed]PrimitiveUP and DrawPrimitiveStrided. Can we fix that? I think I would prefer it if primitiveDeclarationConvertToStridedData() handled all that, and just returned whether we can use the fast draw function, or have to use the slow one.
Re: msxml3: Determine whether to call xsltInit() or not at run time.
Michael Karcher wrote: [...] As a reminder, the issue is that it's not because the libxslt.so we compile against has xlstInit() that the one we load at runtime will have it too. And vice versa. Wait a moment. I don't know about Wine's dl* functions, but can't we use dlsym with the RTLD_DEFAULT handle to look in the libxslt msxml3.dll.so is linked with anyway? That would do away with the need to know the soname during compilation. As it is hardcoded into the executable during link, hard-coding the soname doesn't hurt that much too, but still it gets simpler I tried that but apparently libxslt does not get loaded with RTLD_GLOBAL. So RTLD_DEFAULT did not work :-( But if I missed something and there really is a way it would be great. wine_dl*() is a thin wrapper around the dl*() functions. If you ignore the last two parameters (used for error reporting), then they behave exactly like the standard functions. -- Francois Gouget [EMAIL PROTECTED]
[wdm.h] AddDevice member
Hi,
I've noticed that the AddDevice member of DRIVER_EXTENSION is not declared as
a functio pointer. I've put up a patch to fix this.
Alessandro Pignotti
--
Vi Veri Veniversum Vivus Vici
-Dr. Faustus - Marlowe
Public GPG Key ID 0x650B3ED9 on subkeys.gpg.net
Key Fingerprint 6243 AAD3 E3EC 52D8 DFAA 2A2F 9FCD 0457 650B 3ED9
Encrypted mails are welcome
diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h
index 3e4528b..c8089a8 100644
--- a/include/ddk/wdm.h
+++ b/include/ddk/wdm.h
@@ -300,7 +319,7 @@ typedef struct _DEVICE_OBJECT *PDEVICE_OBJECT;
typedef struct _DRIVER_EXTENSION {
struct _DRIVER_OBJECT *DriverObject;
- PVOID AddDevice;
+ NTSTATUS (*AddDevice)(IN struct _DRIVER_OBJECT* DriverObject, IN struct _DEVICE_OBJECT* PhysicalDeviceObject);
ULONG Count;
UNICODE_STRING ServiceKeyName;
} DRIVER_EXTENSION, *PDRIVER_EXTENSION;
signature.asc
Description: This is a digitally signed message part.
Re: (try 5) comdlg32/tests: Windows XP+ cannot do a CreateViewWindow2 twice in rapid seccesion. handle this error.
Alexandre Julliard wrote: Aric Stewart [EMAIL PROTECTED] writes: @@ -182,19 +182,26 @@ static UINT CALLBACK create_view_window2_hook(HWND dlg, UINT msg, WPARAM wParam, hr = IShellView2_GetCurrentInfo(shell_view2, folder_settings); ok(SUCCEEDED(hr), GetCurrentInfo returned %#x\n, hr); -ok(folder_settings.ViewMode == FVM_LIST, view mode is %d, expected %d\n, folder_settings.ViewMode, FVM_LIST); +ok(folder_settings.ViewMode == FVM_DETAILS, view mode is %d, expected %d\n, folder_settings.ViewMode, FVM_DETAILS); hr = IShellView2_DestroyViewWindow(shell_view2); ok(SUCCEEDED(hr), DestroyViewWindow returned %#x\n, hr); -view_params.pvid = VID_Details; +folder_settings.ViewMode = FVM_LIST; +folder_settings.fFlags = 0; +view_params.pvid = NULL; hr = IShellView2_CreateViewWindow2(shell_view2, view_params); -ok(SUCCEEDED(hr), CreateViewWindow2 returned %#x\n, hr); +/* + * Windows XP is unable to recreate the ViewWindow2 returning + * A Catastrophic failure error + */ So why can't it recreate it? Do you need to clean up something else beforehand? Hi, In an effort to squash the last test failures (W2K3), I had a go at this one. When I add a DestroyWindow() the tests succeed on my W2K3. I'm however not sure it's the correct way or if I'm even close: hr = IShellView2_DestroyViewWindow(shell_view2); ok(SUCCEEDED(hr), DestroyViewWindow returned %#x\n, hr); +DestroyWindow(view_params.hwndView); + view_params.pvid = VID_Details; hr = IShellView2_CreateViewWindow2(shell_view2, view_params); ok(SUCCEEDED(hr), CreateViewWindow2 returned %#x\n, hr); The DestroyWindow() succeeds on W2K3 and Vista but fails (ERROR_INVALID_WINDOW_HANDLE) on W2K. (Even though DestroyWindow() succeeds on Vista is doesn't fix the failure on that platform). -- Cheers, Paul.
Re: implement OleLoadPictureFile
On Thu, 13 Nov 2008, Juan Lang wrote: Any feedback on the spec file or the debug trace? I'm afraid I don't know what substantive difference there is between looking at one small portion of the disassembly (to verify a function is being called) and learning something more substantial. There are certain kinds of reverse engineering that are allowed, but looking at disassembly is not. And now that I know that, I certainly won't be doing it again. I don't think this is going to go in, sorry. Well, I still have an application that won't run under wine because this function is not implemented. So assuming *this* patch doesn't get in, what can I do to help get *a* patch in that implements this function? I can blow away my sandbox and start from scratch, but I suspect you would find that insufficient. Maybe I should find a hypnotist to make me forget what I saw in the debugger :) Would it be acceptable for me to write up a description of what the function needs to do, so that someone else can do a clean-room implementation? It would probably take a reasonably experienced C/COM developer all of about 5 minutes, since this function is really just a wrapper of an already-implemented function. Is there anyone out there who would volunteer to do it if I were to write a description of the function? If I provided a patch to dlls/oleaut32/tests/olepicture.c adding tests that verify the behaviors of the functions, would that be accepted? Or is writing tests for functions you've seen the disassembly for also prohibited? I realize now that I've made a mistake by looking at the function in the debugger, but hopefully there is still a way I can help get this function implemented. Thanks, Jeremy
Re: DIB Engine
В сообщении от Friday 14 November 2008 12:43:48 Roderick Colenbrander написал(а): В сообщении от Thursday 13 November 2008 13:22:07 Massimo Del Fedele написал(а): Sergey Novosyolov ha scritto: I've already started working on it about 3 months ago and released some functions inside DIB Engine. But now we're thinking about release DIB inside GDI32 as Detlef Riekenberg proposed: On 9/29/08, Sergey Novosyolov [EMAIL PROTECTED] wrote: The first thing, i like to see is a Design in the correct way: Inside gdi32 while using Eng* and friends. (Needed by Printer drivers, and any Display driver including mirror / remote display drivers) why can't we release DIB-Engine as own driver? GDI32 functions are main and GDI32 calls driver functions in dependence of which type of DC we have (printer DC, Xwindow HDC or DIB DC) Any Driver can call the Graphic Rendering Engine (GRE) to do parts (or all) of the rendering (and native driver do that): 1: DDB (Driver managed: using any driver specific format) (The Driver should do Everything. When the driver call the GRE, the DDB is converted to a DIB, GDI renders into the DIB and then the DIB is converted back to a DDB) = like our winex11.drv and wineps.drv 2: DDB (GDI managed: using DIB format) (The driver render, what the driver want to render with hw-support and can call the GRE for all the other rendering without converting) = Needed for native printer drivers / mirror drivers or OpenGL accelerated rendering (stefan did some experiments) 3: DIB (GDI renders everything) = The current Code is using a X11-DDB (Driver Managed) with converting issues. So the conception of new strusture of DIB ENgine inside GDI is needed The question is if we should support native drivers or not, other than winex11 or wineps. For winex11, we're using native rendering functions, for wineps we're just translating GRE calls to ps code directly, no need to convert forth and back. Other stuff would be raw printing, for example, using native drivers but are they really needed ? AFAIK the bottleneck now is the double conversion of DIB-X11 DDB-DIB, in order to be able to use X11 rendering functions, so the point 3. I don't understand your point 1; why convert DDB to DIB ? Driver should render directly into DDB. GDI calls can be directed to native ones. I see it this way (but I could be wrong) : 1) Application uses a DIB format, rendering should be done by DIB driver, conversion is needed only to display it. This is what by now is done with 2 conversions between DIB-X11-DIB formats. As i see if we operate with Display DC we no need to convert and GDI32 calls X11DRV functions directly. If we operate with memoryDC GDI calls DIB functions and then uses BitBlt if needed to reflect memoryDC operations on teh display 2) Application uses accelerated opengl, for example; it must (afaik) use native format and functions to render it. No need to convert anything. what do you mean native format? is it connected with GDI calls? 3) Printer drivers. For ps, they're rendered translating GDI calls into postscript code, for other format the driver should do the rendering. Again, no conversion needed. I agree So, I don't understand why to have DIB engine INSIDE GDI. Function pointers could handle the correct driver calls depending on DIB (or DDB) format. DIB is Device Independent Bitmap so DIB functions would be include those functions which implemented the same independ of device Max From reading all your mails I get the impression that Etersoft is also doing some work on the DIB engine. What work has Etersoft done on this area? It might be wise to post the code somewhere for review before the wrong direction is taken again and it might prevent code duplication. Roderick We have received Huw Davies and Jesse Allen gits with their versions of DIB and continued working on it. But we're planning to change DIB Engine structure in these cases: 1. Releasing DIB Engine functions inside GDI. 2. Releasing interactioin DIB Engine with other drivers (such as X11, PS etc.) Today we've developed some DIB functions iwithout cnaging DIB driver structure. The main structure of DIB is just an idea and it was not changed yet.
Re: implement OleLoadPictureFile
On Sun, 16 Nov 2008, Juan Lang wrote: Do you have a bug open? Sorry, I've forgotten. If not, please do open one. You can describe your findings there. No, but a quick search on bugzilla for OleLoadPictureFile turned up bug 10156. A comment on that bug suggests changing the summary to OleLoadPictureFile not implemented, which would exactly sum up my situation. I posted a comment to that bug to that affect. Should that bug be renamed, I could post my findings there... If I provided a patch to dlls/oleaut32/tests/olepicture.c adding tests that verify the behaviors of the functions, would that be accepted? Or is writing tests for functions you've seen the disassembly for also prohibited? In general, writing tests is okay, as they are an exercise in black-box reverse engineering, which is what's allowed. So yeah, tests would be great. I've been looking at writing some tests (I've been compiling using make crosstest and running the exe on windows 2000), and I've found that there must be some sort of mapping between error codes from GetLastError when accessing the file and the HRESULT values returned from OleLoadPicutreFile (and not of the standard HRESULT_FROM_WIN32 variety that I'm familiar with). For instance, CTL_E_FILENOTFOUND is returned when the file does not exist. I'm trying to invoke as many error cases in the test as I can to flesh out this mapping, but I'm not sure I can find all of them in this manner... I think at some point in the next couple of days I'll decide I've got as many as I can and send in a patch to the tests.
Re: implement OleLoadPictureFile
On Mon, 17 Nov 2008, Michael Karcher wrote: The best way to do this is (in my oppinion) submitting the testcase again, but without the implementation, and marking the test todo_wine. A bug might be useful, but wouldn't a mail that contains the testcase as patch and the description in the comment part to wine-patches suffice? Sounds good to me. Be sure to write your findings like (this is hypothetical, as I did not look at your patches and the OlePicture stuff till now): Loads an image from a File. This is just like OleLoadPictureStream, but with a file name instead of a stream as source specification and not like To implement the function, you must create a stream from the file (use the standard OLE file stream object for that), pass the stream to OleLoadPictureStream and finally release the stream. In the error case you should do foo. The second form *is* just publishing what you saw in disassembly, so everyone who reads this mail carefully is in my oppinion everyone reading it carefully is in the same situation as you and shouldn't implement the function. Hmm, looks like I don't really have to. Your detailed description is about 90% accurate :). But the simple version should read This is just like OleLoadPicture... I'll let you extrapolate the change to the detailed version. Note that the IDL defines the VARIANT parameter as optional. If the filename is not specified, you should pass a NULL stream to OleLoadPicture. Also, there is an OleLoadPictureFileEx, which adds the same 3 additional paramters as OleLoadPictureEx adds over OleLoadPicture. You can probably guess how that works... For the non-Ex versions, it may help you to know about the constant LP_DEFAULT in olectl.h, which means do the default thing to the extra params of the Ex version. As far as the standard OLE file stream object, if you know of one please let me know. AFAIK, it is a pretty major oversight in the OLE IStream APIs that there is not a function to create an IStream given a file (name or handle). I've always had to roll my own wrapper around the file APIs, or use a hack like the one in OleLoadPictureFilePath. Testcases on the other hand are fine. They just describe *what* to do, but not *how*. Especially for the error case testcases might be interesting. Quite. Also in olectl.h are the error codes that these functions could return. Unfortunately, I think it will be necessary to make some sort of manual translation between the GetLastError returns from the file APIs and these errors. Without spelling out the mappings from the disassembly, it will probably be nearly impossible to get all of the cases exactly right. The tests I have come up with so far verify the error codes that I've been able to figure out how to trigger. Is there anyone out there who would volunteer to do it if I were to write a description of the function? I would do so, if its just some minutes. I think that, as long as you are familiar with the APIs for dealing with VARIANTs, this should be a very simple task. I would provide details about how to deal with the VARIANT, but it may be misinterpreted as knowledge gained from the disassembler. So I'll just leave you with a couple of links to MS docs. http://msdn.microsoft.com/en-us/library/ms221673.aspx http://support.microsoft.com/kb/238981
Re: Patch for [Bug 16087] wine spews innumerable Unable to check compatibility for Format... errors in some games
Your patch adds extra whitespace on an empty line. You are mixing coding styles with your if statements. When sending a patch to wine-patches put the patch number followed by how many patches you are sending so the automated scripts can read it. For example [1/1] sending only one patch. 2008/11/16 Bogdan Butnaru [EMAIL PROTECTED]: Patch for http://bugs.winehq.org/show_bug.cgi?id=16087 -- Bogdan Butnaru
RE: [1/5] WineD3D: Get some vertex pipeline infrastructure in place
Can we fix that? I think I would prefer it if primitiveDeclarationConvertToStridedData() handled all that, and just returned whether we can use the fast draw function, or have to use the slow one. We never call primitiveDeclarationConverToStridedData() with drawPrimitiveStrided. We could of course add a function that checks the application provided strided data(ie, the very if statement we have here), but I don't think that gains us a lot
re: Patch for [Bug 16087] wine spews innumerable Unable to check compatibility for Format... errors in some games
Andrew wrote: When sending a patch to wine-patches put the patch number followed by how many patches you are sending so the automated scripts can read it. yes, please! For example [1/1] sending only one patch. Poor example, please don't do it on single patches. You only need to do it for series of more than one patch. Also please don't send patches that don't apply cleanly against current git. (You'd be surprised how many people do this. It's ok if Alexandre can figure it out, but poor old Patchwatcher will reject it.) - Dan
Re: implement OleLoadPictureFile
Am Montag, den 17.11.2008, 13:09 -0800 schrieb Jeremy Drake: Be sure to write your findings like (this is hypothetical, as I did not look at your patches and the OlePicture stuff till now): Loads an image from a File. This is just like OleLoadPictureStream, but with a file name instead of a stream as source specification and not like To implement the function, you must create a stream from the file (use the standard OLE file stream object for that), pass the stream to OleLoadPictureStream and finally release the stream. In the error case you should do foo. The second form *is* just publishing what you saw in disassembly, so everyone who reads this mail carefully is in my oppinion everyone reading it carefully is in the same situation as you and shouldn't implement the function. Hmm, looks like I don't really have to. Your detailed description is about 90% accurate :). But the simple version should read This is just like OleLoadPicture... I'll let you extrapolate the change to the detailed version. I just guessed from the text that accompanied your patch and the function names. Note that the IDL defines the VARIANT parameter as optional. If the filename is not specified, you should pass a NULL stream to OleLoadPicture. How do I know? I think you shouldn't have told these detail. I might know what happens if I pass in VARIANTs of type VT_ERROR or VT_NULL, like lplpdispPicture is unchanged, set to NULL or Windows crashes. But there seems no legal way for me to find out what happens inside Microsoft's oleaut32.dll. If it were an inter-dll call, relay or snoop might bring me to the conclusion that you are right, but I just pretend I never read that. Just write a couple of tests for different variant types (VT_NULL, VT_ERROR, VT_BSTR). If they crash on Windows, put them in an if(0) block with an appropriate comment. With wine, it will crash in olepicture.c:1787 if I pass a NULL stream into OleLoadPicture. If it crashes in windows too, it does not matter how we implement the crash in Wine. I don't even think that we have to crash everything that crashes in Windows. And if it will crash, I might just as well assume that the VARIANT is a VT_BSTR without any checks, as no Win32 program is going to call with wrong parameters. You seem to have looked at the disassembled code too much to see that there is more than one way to do it. But exactly this seperates a clean-room implementation from a rip-off. The clean-room reimplementers (which could be me) must not be provided with the way how to do it, so they can find there own way (this is the creative power that makes wine our own work!) instead of blindly following the way Microsoft decided to use. Also, there is an OleLoadPictureFileEx, which adds the same 3 additional paramters as OleLoadPictureEx adds over OleLoadPicture. You can probably guess how that works... Do a three-way-merge of OleLoadPicture to OleLoadPictureEx and OleLoadPicture to OleLoadPictureFile and you arrive at OleLoadPictureFileEx, probably. For the non-Ex versions, it may help you to know about the constant LP_DEFAULT in olectl.h, which means do the default thing to the extra params of the Ex version. MSDN documents LP_DEFAULT only for the palette in OleLoadPictureFileEx, but follows your points in the documentation for OleLoadPictureEx. It might have been that the the desired size should be just zero (which LP_DEFAULT happens to be) to get the default. As far as the standard OLE file stream object, if you know of one please let me know. AFAIK, it is a pretty major oversight in the OLE IStream APIs that there is not a function to create an IStream given a file (name or handle). Oops. I just assumed there was one, but you seem to be right. One could try whether the Windows FileMoniker works on objects that don't implement IPersistFile but just IPersistStream and thus IMoniker::BindToObject from the FileMoniker does what we want. But this is a very dangerous approach, as you can't tell BindToObject the CLSID to use, but just the IID you want, and the Wine implementation will happily activate and load *any* file type you pass in, and notice after loading that the object loaded does not support the IPicture interface. I don't know what ugly things can happen on IPersistStream::Load given some CLSIDs no one thinks about in the moment (maybe XML documents that cause an internet access to fetch the DTD), but I think some Media Player/Outlook security vulnerabilities we had some years ago are related to implementing things like this (that was IIRC attachments of the name .WAV that are associated with windows media player; windows media player checked magic numbers in that file, completely forgetting about the file a wave file finding MZ in the beginning and acting appropriately). You will see whether OLELoadPictureFile does that if you do a relay trace with native oleaut32 and builtin ole32, as the CreateFileMoniker call would cross DLL boundaries.
Re: [PATCH 1/2] Implement GdipGetLogFontA (try3)
Was there something wrong with this patch series?
On Tue, 2008-11-11 at 11:24 -0500, Adam Petaccia wrote:
Changelog:
try3:
Only copy the bytes we'll use
Toss out bytesWritten
try2:
Fix memcpy() use
Add documentation
Add testcase
---
dlls/gdiplus/font.c | 27 +++
dlls/gdiplus/gdiplus.spec |2 +-
include/gdiplusflat.h |1 +
3 files changed, 29 insertions(+), 1 deletions(-)
diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c
index 32713c7..7f815b6 100644
--- a/dlls/gdiplus/font.c
+++ b/dlls/gdiplus/font.c
@@ -368,6 +368,33 @@ GpStatus WINGDIPAPI GdipGetFontUnit(GpFont *font, Unit
*unit)
}
/***
+ * GdipGetLogFontA [EMAIL PROTECTED]
+ *
+ * Converts a font into a LOGFONTA
+ *
+ * PARAMS
+ * font[I] GpFont to create LOGFONTA object
+ * graphics[I] Current graphics context
+ * lfa [O] Resulting LOGFONTA object
+ * RETURNS
+ * InvalidParameter: font, graphics, or lfa was NULL.
+ * Ok: otherwise
+ */
+GpStatus WINGDIPAPI GdipGetLogFontA(GpFont *font, GpGraphics *graphics,
+LOGFONTA *lfa)
+{
+/* FIXME: use graphics */
+if(!(font graphics lfa))
+return InvalidParameter;
+
+memcpy(lfa, font-lfw, FIELD_OFFSET(LOGFONTA, lfFaceName));
+WideCharToMultiByte(CP_ACP, 0, font-lfw.lfFaceName, -1,
+lfa-lfFaceName, LF_FACESIZE, NULL, NULL);
+
+return Ok;
+}
+
+/***
* GdipGetLogFontW [EMAIL PROTECTED]
*/
GpStatus WINGDIPAPI GdipGetLogFontW(GpFont *font, GpGraphics *graphics,
diff --git a/dlls/gdiplus/gdiplus.spec b/dlls/gdiplus/gdiplus.spec
index c25d4b7..dce558c 100644
--- a/dlls/gdiplus/gdiplus.spec
+++ b/dlls/gdiplus/gdiplus.spec
@@ -311,7 +311,7 @@
@ stdcall GdipGetLineSpacing(ptr long ptr)
@ stub GdipGetLineTransform
@ stdcall GdipGetLineWrapMode(ptr ptr)
-@ stub GdipGetLogFontA
+@ stdcall GdipGetLogFontA(ptr ptr ptr)
@ stdcall GdipGetLogFontW(ptr ptr ptr)
@ stdcall GdipGetMatrixElements(ptr ptr)
@ stub GdipGetMetafileDownLevelRasterizationLimit
diff --git a/include/gdiplusflat.h b/include/gdiplusflat.h
index ea4bec5..a2c6fc9 100644
--- a/include/gdiplusflat.h
+++ b/include/gdiplusflat.h
@@ -81,6 +81,7 @@ GpStatus WINGDIPAPI GdipCreateFontFromDC(HDC,GpFont**);
GpStatus WINGDIPAPI GdipCreateFontFromLogfontA(HDC,GDIPCONST
LOGFONTA*,GpFont**);
GpStatus WINGDIPAPI GdipCreateFontFromLogfontW(HDC,GDIPCONST
LOGFONTW*,GpFont**);
GpStatus WINGDIPAPI GdipDeleteFont(GpFont*);
+GpStatus WINGDIPAPI GdipGetLogFontA(GpFont*,GpGraphics*,LOGFONTA*);
GpStatus WINGDIPAPI GdipGetLogFontW(GpFont*,GpGraphics*,LOGFONTW*);
GpStatus WINGDIPAPI GdipGetFamily(GpFont*, GpFontFamily**);
GpStatus WINGDIPAPI GdipGetFontUnit(GpFont*, Unit*);
Re: implement OleLoadPictureFile
2008/11/17 Michael Karcher [EMAIL PROTECTED]: Am Montag, den 17.11.2008, 13:09 -0800 schrieb Jeremy Drake: As far as the standard OLE file stream object, if you know of one please let me know. AFAIK, it is a pretty major oversight in the OLE IStream APIs that there is not a function to create an IStream given a file (name or handle). Oops. I just assumed there was one, but you seem to be right. In shlwapi there is a SHCreateStreamFromFile with A, W and Ex versions when given a filename. Quite. Also in olectl.h are the error codes that these functions could return. Unfortunately, I think it will be necessary to make some sort of manual translation between the GetLastError returns from the file APIs and these errors. Without spelling out the mappings from the disassembly, it will probably be nearly impossible to get all of the cases exactly right. That is fair. Even different Windows versions are some times inconsistent with their error codes. It is more important what conditions exactly count as errors, and what happens to the data pointed to by lplpdispPicture parameter (unchanged / set to NULL / set to a valid object implementing IDispatch and IPicture). Most prominent error codes like File not found and File is not in any supported format should be checked, but that's it. You shouldn't even have told that the mapping is done directly in OleLoadPictureFile and not some lower layer invoked by OleLoadPictureFile, except if you are sure from a relay trace. Of course, if Wine developers find that *somewhere* error codes seem to be mapped, they are free to do the mapping in OleLoadPictureFile. There should not be any mapping of error codes, except where tests clearly show what is returned and that does not match what is returned by the APIs being called to implement the function. Some of the errors - such as file not found can be handled by the called APIs like SHCreateStreamOnFile. There should still be tests for these codepaths as well. FWIW, applications are not usually concerned with the specifics of why an API failed, just that it did. This, in addition to Windows changing the return codes every release means that the error codes returned won't make that big a difference; the only error codes you can rely on will be those listed in the MSDN documentation. - Reece
Re: [PATCH 1/2] Implement GdipGetLogFontA (try3)
I can't say for sure, but I think you should share code with
GdipGetLogFontW somehow (probably either calling it or adding a helper
for both functions to call).
Vincent Povirk
On Mon, Nov 17, 2008 at 6:47 PM, Adam Petaccia [EMAIL PROTECTED] wrote:
Was there something wrong with this patch series?
On Tue, 2008-11-11 at 11:24 -0500, Adam Petaccia wrote:
Changelog:
try3:
Only copy the bytes we'll use
Toss out bytesWritten
try2:
Fix memcpy() use
Add documentation
Add testcase
---
dlls/gdiplus/font.c | 27 +++
dlls/gdiplus/gdiplus.spec |2 +-
include/gdiplusflat.h |1 +
3 files changed, 29 insertions(+), 1 deletions(-)
diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c
index 32713c7..7f815b6 100644
--- a/dlls/gdiplus/font.c
+++ b/dlls/gdiplus/font.c
@@ -368,6 +368,33 @@ GpStatus WINGDIPAPI GdipGetFontUnit(GpFont *font, Unit
*unit)
}
/***
+ * GdipGetLogFontA [EMAIL PROTECTED]
+ *
+ * Converts a font into a LOGFONTA
+ *
+ * PARAMS
+ * font[I] GpFont to create LOGFONTA object
+ * graphics[I] Current graphics context
+ * lfa [O] Resulting LOGFONTA object
+ * RETURNS
+ * InvalidParameter: font, graphics, or lfa was NULL.
+ * Ok: otherwise
+ */
+GpStatus WINGDIPAPI GdipGetLogFontA(GpFont *font, GpGraphics *graphics,
+LOGFONTA *lfa)
+{
+/* FIXME: use graphics */
+if(!(font graphics lfa))
+return InvalidParameter;
+
+memcpy(lfa, font-lfw, FIELD_OFFSET(LOGFONTA, lfFaceName));
+WideCharToMultiByte(CP_ACP, 0, font-lfw.lfFaceName, -1,
+lfa-lfFaceName, LF_FACESIZE, NULL, NULL);
+
+return Ok;
+}
+
+/***
* GdipGetLogFontW [EMAIL PROTECTED]
*/
GpStatus WINGDIPAPI GdipGetLogFontW(GpFont *font, GpGraphics *graphics,
diff --git a/dlls/gdiplus/gdiplus.spec b/dlls/gdiplus/gdiplus.spec
index c25d4b7..dce558c 100644
--- a/dlls/gdiplus/gdiplus.spec
+++ b/dlls/gdiplus/gdiplus.spec
@@ -311,7 +311,7 @@
@ stdcall GdipGetLineSpacing(ptr long ptr)
@ stub GdipGetLineTransform
@ stdcall GdipGetLineWrapMode(ptr ptr)
-@ stub GdipGetLogFontA
+@ stdcall GdipGetLogFontA(ptr ptr ptr)
@ stdcall GdipGetLogFontW(ptr ptr ptr)
@ stdcall GdipGetMatrixElements(ptr ptr)
@ stub GdipGetMetafileDownLevelRasterizationLimit
diff --git a/include/gdiplusflat.h b/include/gdiplusflat.h
index ea4bec5..a2c6fc9 100644
--- a/include/gdiplusflat.h
+++ b/include/gdiplusflat.h
@@ -81,6 +81,7 @@ GpStatus WINGDIPAPI GdipCreateFontFromDC(HDC,GpFont**);
GpStatus WINGDIPAPI GdipCreateFontFromLogfontA(HDC,GDIPCONST
LOGFONTA*,GpFont**);
GpStatus WINGDIPAPI GdipCreateFontFromLogfontW(HDC,GDIPCONST
LOGFONTW*,GpFont**);
GpStatus WINGDIPAPI GdipDeleteFont(GpFont*);
+GpStatus WINGDIPAPI GdipGetLogFontA(GpFont*,GpGraphics*,LOGFONTA*);
GpStatus WINGDIPAPI GdipGetLogFontW(GpFont*,GpGraphics*,LOGFONTW*);
GpStatus WINGDIPAPI GdipGetFamily(GpFont*, GpFontFamily**);
GpStatus WINGDIPAPI GdipGetFontUnit(GpFont*, Unit*);
Re: dlls/cabinet: fix dead stores (llvm/clang)
ricardo filipe [EMAIL PROTECTED] wrote: --- a/dlls/cabinet/fci.c +++ b/dlls/cabinet/fci.c @@ -462,7 +462,7 @@ static cab_ULONG fci_get_checksum(const void *pv, UINT cb, CHECKSUM seed) case 2: ul |= (((ULONG)(*pb++)) 8); case 1: - ul |= *pb++; + ul |= *pb; default: break; } Is there any reason that you ignored 2 similar cases above? -- Dmitry.
