August 2007 - Wine-Devel - list.winehq.org

Re: wine.inf: Remove RelayFromExclude. It hides the real call traces which makes nearly useless relay logs. [resend]

by Alexandre Julliard

Vitaliy Margolen <wine-patches(a)kievinfo.com> writes: > Resend. Anything wrong with this patch? > > After this change went in, it became impossible to see what functions was > being called where and how inside Wine to understand what is happening. If > any one looks at the bugzilla and all of the reported +relay logs they are > 100% useless in determining the real cause of the problem. It wasn't the > case before without RelayFromExclude. In most cases it makes the trace much more readable and manageable. If you want to know what's going on inside Wine code there are better debug channels to use than relay. -- Alexandre Julliard julliard(a)winehq.org

Aug. 6, 2007

Re: Can we please have at least a minimal coding style ?

by Stephan Rose

On Mon, 2007-08-06 at 10:37 +0200, Roderick Colenbrander wrote: > > Or even worse (I've seen this in winex11.drv, and it took me quite a > > long time until I understood it - it was part of a larger block with a > > lot these constructs): > > > > if (cond) do_sth(); > > do_sth_else(); > > > > I think you are speaking about the opengl code there. There are quite a number > of pieces like that left. They are remnants of the old opengl code. During > the move to winex11.drv I fixed the indentation in those functions a bit as > before it was a lot worse. You had (and still have) perhaps 10 layers of > indentation in some functions but at that time the indentation wasn't there. Easy solution to that. I'm in absolute heaven with this feature... Eclipse 3.3 with CDT has an automatic code formatting feature, Ctrl +Shift+F, my favorite keyboard shortcut now. =) May want to load some files into that and run that over the file to instantly clean up stuff like that. You can tweak the code style to your liking too. Stephan

Aug. 6, 2007

Re: Can we please have at least a minimal coding style ?

by Roderick Colenbrander

> Or even worse (I've seen this in winex11.drv, and it took me quite a > long time until I understood it - it was part of a larger block with a > lot these constructs): > > if (cond) do_sth(); > do_sth_else(); > I think you are speaking about the opengl code there. There are quite a number of pieces like that left. They are remnants of the old opengl code. During the move to winex11.drv I fixed the indentation in those functions a bit as before it was a lot worse. You had (and still have) perhaps 10 layers of indentation in some functions but at that time the indentation wasn't there. Roderick

Aug. 6, 2007

Re: Can we please have at least a minimal coding style ?

by H. Verbeet

On 06/08/07, Tomas Carnecky <tom(a)dbservice.com> wrote: > This type of construct seems popular in the wine source: > > while (isspace(*GL_Extensions)) GL_Extensions++; > Start = GL_Extensions; > Other than the questionable variable naming I see nothing wrong with that construction? > Or even worse (I've seen this in winex11.drv, and it took me quite a > long time until I understood it - it was part of a larger block with a > lot these constructs): > > if (cond) do_sth(); > do_sth_else(); > That one is slightly misleading of course.

Aug. 6, 2007

Re: user32 - set_active_window uses SendMessage instead ofPostMessage for WM_ACTIVATEAPP messages.

by Peter Dons Tychsen

On Mon, 2007-08-06 at 01:26 +0200, Peter Dons Tychsen wrote: > I will go back and redo the tests with this approach, now that you have > torpedoed my idea, which i probably deserved. OK. To recover from my earlier brain-dead idea, i think i have came up with a much simpler solution, based on your recommendations. Please review this idea. The patch adds support for checking posted messages by emptying the queue, and marking all dispatched messages as sent instead of posted. As soon as it is introduced however, it will breaks some test-cases which then needs fixing, as more messages are checked. Not sure about the workload in this. Unfortunately this does seem to indicate that my original idea that WM_ACTIVATEAPP should be posted is wrong (scratch that silly patch). Sorry for the mess. :-) /Pedro

Aug. 6, 2007

Re: wined3d: Use a safer, more compliant method to parse extension strings

by Chris Robinson

On Sunday 05 August 2007 02:23:11 pm Vitaliy Margolen wrote: > Chris Robinson wrote: > > + > > + /* End */ > > + {NULL, 0} > > }; > > [..] > > > - for (i = 0; i < (sizeof(EXTENSION_MAP) / > > sizeof(*EXTENSION_MAP)); ++i) { + for (i = 0; > > EXTENSION_MAP[i].extension_string; ++i) { > > What was the reason for this change? It's a static const array why do you > want to treat it as a dynamically sized one? Because at the time, I was attempting to track down memory corruption, and I didn't trust that the compiler wasn't adding an extra element to the list (due to the last specified element ending with a comma to signify another, and the array size not being explicit). Ending the array with a NULL would gaurantee you can't overrun it in either case.

Aug. 6, 2007

Re: user32 - set_active_window uses SendMessage instead ofPostMessage for WM_ACTIVATEAPP messages.

by Peter Dons Tychsen

On Sun, 2007-08-05 at 21:13 +0200, Peter Dons Tychsen wrote: > On Sun, 2007-08-05 at 21:12 +0200, Peter Dons Tychsen wrote: > > On Sat, 2007-08-04 at 12:04 +0900, Dmitry Timoshkov wrote: > > > "Peter Dons Tychsen" <donpedro(a)tdcadsl.dk> wrote: > > > > > > > 1) Yes i did testing on Windows-XP. I did it by putting together various > > > > examples and by checking with InSendMessage() for all cases. This > > > > clearly showed that WM_ACTIVATEAPP was always posted and never sent. > > > > > > Looks like you misunderstood the InSendMessage behaviour and that led to > > > some confusion. > > > > > > Wine test harness for message sequences works just fine, there is no need > > > to blame it. > > > > > > > I have found a new way of proving that ACTIVATEAPP must be sent using > > the PostMessage call and not the SendMessage call. I have also found a > > way to extend the unit test system to cover this. > > > > After realizing my bummer with InSendMessage() i set out to create a new > > function that would do what i want: Check if a message was sent with > > PostMessage() or SendMessage() on windows. I could not find any function > > that would do this so i made my own hack (below): > > > > This piece of code, if inserted into the test system, could (on original > > Windows) show us which messages we are incorrectly posting or sending. I > > can probably also be used for other test purposes. I works by checking > > the call stack for the calls SendMessageA() and SendMessageW(). Works > > like charm on Windows-XP. Will need some work to also work inside Wine, > > but that is not important for now. > > > > Usage: > > > > 1) Call call_stack_check_init(). > > 2) Call called_from_send_message() inside the Proc handler. > > > > Cheers, > > > > /Pedro > > > > ------ CODE BEGIN -------- > > > > void call_stack_check_init(void) > > { > > HANDLE process = GetCurrentProcess(); > > SymSetOptions(SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS); > > SymInitialize(process, NULL, TRUE); > > } > > > > #define SYM_MAX_NAME_SIZE 1000 > > BOOL call_stack_check_search(char *func) > > { > > STACKFRAME64 call_stack; > > CONTEXT context; > > HANDLE thread = GetCurrentThread(); > > SYMBOL_INFO *sym = (SYMBOL_INFO *) malloc(sizeof(SYMBOL_INFO) + > > SYM_MAX_NAME_SIZE); > > CHAR name[SYM_MAX_NAME_SIZE]; > > HANDLE process = GetCurrentProcess(); > > > > /* Setup symbol buffer */ > > ZeroMemory(sym, sizeof(SYMBOL_INFO) + SYM_MAX_NAME_SIZE); > > sym->SizeOfStruct = sizeof(SYMBOL_INFO); > > sym->MaxNameLen = SYM_MAX_NAME_SIZE; > > > > /* Get thread context */ > > ZeroMemory(&context, sizeof(context)); > > context.ContextFlags = CONTEXT_FULL; > > __asm call x; > > __asm x: pop eax; > > __asm mov context.Eip, eax; > > __asm mov context.Ebp, ebp; > > __asm mov context.Esp, esp; > > > > /* Setup stack walk offset */ > > ZeroMemory(&call_stack, sizeof(call_stack)); > > call_stack.AddrPC.Offset = context.Eip; > > call_stack.AddrStack.Offset = context.Esp; > > call_stack.AddrFrame.Offset = context.Ebp; > > call_stack.AddrPC.Mode = AddrModeFlat; > > call_stack.AddrStack.Mode = AddrModeFlat; > > call_stack.AddrFrame.Mode = AddrModeFlat; > > > > while(1) > > { > > DWORD64 disp = 0; > > > > /* Walk that rusty old stack... */ > > if(!StackWalk64( > > IMAGE_FILE_MACHINE_I386, > > process, > > thread, > > &call_stack, > > NULL, > > NULL, > > SymFunctionTableAccess64, > > SymGetModuleBase64, > > NULL)) return FALSE; > > > > /* Get the next symbol on stack */ > > if(SymFromAddr(GetCurrentProcess(), call_stack.AddrPC.Offset, &disp, > > sym)) > > { > > /* Make it readable */ > > UnDecorateSymbolName(sym->Name, name, SYM_MAX_NAME_SIZE, > > UNDNAME_COMPLETE | > > UNDNAME_NO_THISTYPE | > > UNDNAME_NO_SPECIAL_SYMS | > > UNDNAME_NO_MEMBER_TYPE | > > UNDNAME_NO_MS_KEYWORDS | > > UNDNAME_NO_ACCESS_SPECIFIERS); > > > > /* Check for match */ > > if(!strcmp(func, name)) return TRUE; > > } > > } > > > > return FALSE; > > } > > BOOL called_from_send_message(void) > > { > > if (call_stack_check_search("SendMessageW")) return TRUE; > > if (call_stack_check_search("SendMessageA")) return TRUE; > > > > return FALSE; > > } > > > > ----- CODE END ----- > > By the way, this code does prove (if run on Windows-XP), that > ACTIVATEAPP should be posted and not sent. > > /pedro Oh dear... scrap that last comment. With the comments from Alexandre this no longer valid. It does not prove anything... back to the drawing board Peter... :-) /Pedro

Aug. 6, 2007

Can we please have at least a minimal coding style ?

by Tomas Carnecky

This type of construct seems popular in the wine source: while (isspace(*GL_Extensions)) GL_Extensions++; Start = GL_Extensions; Or even worse (I've seen this in winex11.drv, and it took me quite a long time until I understood it - it was part of a larger block with a lot these constructs): if (cond) do_sth(); do_sth_else(); This indentation is so utterly stupid, it makes my brain hurt (I'm sure I'm not the only one)!! I don't know what AJs stance is on code readability and I know that there are no official coding style guidelines, but can we please avoid this? Can we agree on a set of fundamental style rules? To make peer-reviewing patches easier? And for people not familiar with the code easier to understand it? Please? tom

Aug. 5, 2007

Re: user32 - set_active_window uses SendMessage instead ofPostMessage for WM_ACTIVATEAPP messages.

by Peter Dons Tychsen

On Sun, 2007-08-05 at 23:23 +0200, Alexandre Julliard wrote: > Peter Dons Tychsen <donpedro(a)tdcadsl.dk> writes: > > > This piece of code, if inserted into the test system, could (on original > > Windows) show us which messages we are incorrectly posting or sending. I > > can probably also be used for other test purposes. I works by checking > > the call stack for the calls SendMessageA() and SendMessageW(). Works > > like charm on Windows-XP. Will need some work to also work inside Wine, > > but that is not important for now. > > This is silly. It can't possibly work right since there is no reason > that messages are sent using the exported functions, it won't work on > anything not compiled with frame pointer, and there's absolutely no > need for anything like that at all. Hello again, For the tests i am running there is always a frame pointer. You might be right that there are other (better) ways to solve this problem, but is still think this little function could come in handy when black-box testing original DLLs. You are of course right about the point of the exported functions. It slipped my mind... I am not sure the word "silly" applies however... :-). > A posted message doesn't call the > window proc directly, it goes through the message queue, so it's > completely trivial to check for it between GetMessage and > DispatchMessage. In this case we would need to use PeekMessage(), but yes, you are right. I scrapped this solution to begin with as it required too much many changes in the test suite (and probably also because i liked the more complicated version - should have stuck with the simple one :-)) I always try to follow the "keep it simple" rule-sets, but it is not always i succeed in doing it... :-(. I will go back and redo the tests with this approach, now that you have torpedoed my idea, which i probably deserved. Thanks, /Pedro

Aug. 5, 2007

Re: [1/5] D3D9: Deal with failing surface creation

by Stefan Dösinger

Am Freitag, 3. August 2007 18:18 schrieb H. Verbeet: > On 03/08/07, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > > + ok(SUCCEEDED(hr) || hr == D3DERR_INVALIDCALL, > > "IDirect3DDevice9_CreateTexture: %s\n", DXGetErrorString9(hr)); > > Same comment as for yesterday's test (even though that one apparently > made it in), I don't think that's a useful test. As the patch comment > says, the point of the test isn't to test CreateTexture behaviour. Yes, but on the other hand it does not cost much. I still prefer to have tests which are on the first sight useless. There may still be some windows driver which returns an unexpected value. Even if we don't do much about it, except changing the test to not bail out, it may be of use(e.g. a game doesn't work on that driver and the vendor says this happens because the card is not fully directx9c capable). The tests documents a tiny, neglicable bit of information, but it still documents something. If the CreateTexture call was the core of that test I'd check the device caps, check if the pixel format is supported and check the return value based on the supported formats.

Aug. 5, 2007