Re: inetmib1(3/31): Add tests for SnmpExtensionInit
Juan Lang wrote: --Juan These tests import snmpapi, which isn't available on win95. Do we need to cater for that and do LoadLibrary/GetProcAddress ? -- Cheers, Paul.
App non running with +relay enabled
What could be the reason of an app running fine with no relay enabled, but exiting very soon with it enabled ? Best Regards Max
Re: App non running with +relay enabled
Am Donnerstag, den 26.06.2008, 10:37 +0200 schrieb Massimo: What could be the reason of an app running fine with no relay enabled, but exiting very soon with it enabled ? Different reasons. - A debugger detection that detects the relay thunks as debugger. Some copy protection schemes are a bit picky. - A race condition in the application that gets exploited by the big overhead of relay dumping. - Something times out when doing relay traces. - Use of uninitialized stack variables in. The contents of uninitialized variables can be very different with and without relay tracing. The bug might be in wine (forgetting to initialize something that must be initialized) or in a callback function of the application that by accident expects a zero in an uninitialized variable. - A bug in wine that causes a crash in relay dumping code (only if you see a backtrace at the end of the log; on the other hand, relay seems like a mostly safe channel, as it does not print user data except for strings, and that codepath is very well exercised.) Regards, Michael Karcher
RE: opengl32: work around a crash in glGetString
Does this crash occur with Mesa as well? If yes, it might be interesting to step through the Mesa code to see what is going wrong in there. Otherwise, drivers are never bug free of course. This could be a bug in the Nvidia driver From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Louis. Lenders Sent: Thursday, June 26, 2008 9:06 AM To: [EMAIL PROTECTED] Subject: opengl32: work around a crash in glGetString Hi this fixes http://bugs.winehq.org/show_bug.cgi?id=13599 A +relay trace shows glGetString is crashing inside glGetString. The cause is _not_ a lacking opengl context setup around glGetString. I did some testing and both on windows and wine glGetString just returns NULL if this is the case, but it never crashes inside glGetString: /* glGetString without opengl context: */ 0009:Call opengl32.glGetString(1f01) ret=004016bc 0009:Ret opengl32.glGetString() retval= ret=004016bc /* Crash from Autocad */ 0064:Call opengl32.glGetString(1f01) ret=1000cd58 0064:trace:seh:raise_exception code=c005 flags=0 addr=0x60ec12f6 0064:trace:seh:raise_exception info[0]= /* and never returns*/ This bug has been reproduced by several other people , so it's also not due to a bad gl-setup on my side. In case you want to try , Autocad trial 2004 can be downloaded from http://www.brothersoft.com/autocad-78351.html. It's the main issue that prevent autocad 2004 up to 2008 from running. So for now my conclusion is, this is a nasty bug outside wine, and an exception handler is needed to work around the crash. If the patch is rejected i would appreciate some input. Thanks and regards _ Not happy with your email address? Get the one you really want http://uk.docs.yahoo.com/ymail/new.html - millions of new email addresses available now at Yahoo! http://uk.docs.yahoo.com/ymail/new.html
Re: opengl32: work around a crash in glGetString
As I mentioned before this requires a test case. I remember from the gl log that the program didn't create a WGL context at all. It isn't allowed to make opengl calls because of that. You should figure out why that is done. Either some other check in the program failed due to which it wasn't able to create a context OR the application is looking for some strange behavior. Perhaps without a context (if Windows allows it) you get some GDI software renderer string back or so. Anyway at this point it sounds like a workaround instead of a fix for the real bug. Roderick Hi this fixes http://bugs.winehq.org/show_bug.cgi?id=13599 A +relay trace shows glGetString is crashing inside glGetString. The cause is _not_ a lacking opengl context setup around glGetString. I did some testing and both on windows and wine glGetString just returns NULL if this is the case, but it never crashes inside glGetString: /* glGetString without opengl context: */ 0009:Call opengl32.glGetString(1f01) ret=004016bc 0009:Ret opengl32.glGetString() retval= ret=004016bc /* Crash from Autocad */ 0064:Call opengl32.glGetString(1f01) ret=1000cd58 0064:trace:seh:raise_exception code=c005 flags=0 addr=0x60ec12f6 0064:trace:seh:raise_exception info[0]= /* and never returns*/ This bug has been reproduced by several other people , so it's also not due to a bad gl-setup on my side. In case you want to try , Autocad trial 2004 can be downloaded from http://www.brothersoft.com/autocad-78351.html. It's the main issue that prevent autocad 2004 up to 2008 from running. So for now my conclusion is, this is a nasty bug outside wine, and an exception handler is needed to work around the crash. If the patch is rejected i would appreciate some input. Thanks and regards __ Not happy with your email address?. Get the one you really want - millions of new email addresses available now at Yahoo! http://uk.docs.yahoo.com/ymail/new.html -- Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer
Re: quartz Rewrite the avi splitter so that video and sound run in separate threads, with tests
Maarten Lankhorst [EMAIL PROTECTED] writes: +FIXME(Waiting for %u to terminate\n, x); +/* Make the thread return first */ +SetEvent(stream-packet_queued); +if (WaitForSingleObject(stream-thread, 1) == WAIT_TIMEOUT) +exit(1); Don't use exit(). If you absolutely need to kill the whole process you should use ExitProcess, and you have to print an error so that users know what happened. But I doubt you need such a drastic action here. -- Alexandre Julliard [EMAIL PROTECTED]
Whats wrong with my patch.
Hi! I sent a patch last Friday to the wine-patches list. I think that I fulfill the rules that I read on winehq website. Patch is simple implementation of function GdipDrawEllipse, and is based on existing function GdipFillEllipse. This is my first patch to the Wine project. I don't have deeper Wine, and Windows knowledge, so I ask, what else I must do, to get this patch accepted. The link to the mail is below: http://www.winehq.org/pipermail/wine-patches/2008-June/056223.html -- Best Regards Przemysław Białek
Re: Memory leak patch
On Thu, Jun 26, 2008 at 03:25:30AM -0700, Joris Huizer wrote: dlls/gdiplus/font.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 9ef8edc..86122da 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -42,6 +42,7 @@ static inline REAL get_dpi (void) HDC hdc = GetDC(0); GdipCreateFromHDC (hdc, graphics); GdipGetDpiX(graphics, dpi); +GdipFree(graphics); ReleaseDC (0, hdc); return dpi; This should be GdipDeleteGraphics. Huw. -- Huw Davies [EMAIL PROTECTED]
opengl32: work around a crash in glGetString
As I mentioned before this requires a test case. As i tried to explain , in a simple test glGetString just return null, it will not crash for me, like it does in the program. I remember from the gl log that the program didn't create a WGL context at all. It isn't allowed to make opengl calls because of that Allowed or not, the program does so, and on windows it installs just fine. Perhaps without a context (if Windows allows it) you get some GDI software renderer string back or so. It just returns (null) Anyway at this point it sounds like a workaround instead of a fix for the real bug. Well, the bug might be outside wine, so then it would be workaround, yes, but still necessary IMO. I cannot see how i can make a testcase based on the output of autocad here: http://bugs.winehq.org/attachment.cgi?id=14005 , other then i did already. It's just an endless sequence of 'wglGetIntegerv','wglFinish', and 'wglFlush', and then a crash in glGetString. I cannot make a test program crash like that; the test program just returns null, like i said before A last stupid question for stefan: how can i make X load the mesa driver, instead of my nvidia? __ Not happy with your email address?. Get the one you really want - millions of new email addresses available now at Yahoo! http://uk.docs.yahoo.com/ymail/new.html
Re: Whats wrong with my patch.
Przemysław Białek wrote: Hi! I sent a patch last Friday to the wine-patches list. I think that I fulfill the rules that I read on winehq website. Patch is simple implementation of function GdipDrawEllipse, and is based on existing function GdipFillEllipse. This is my first patch to the Wine project. I don't have deeper Wine, and Windows knowledge, so I ask, what else I must do, to get this patch accepted. The link to the mail is below: http://www.winehq.org/pipermail/wine-patches/2008-June/056223.html Hi, the only question for me is why don't you select NULL_BRUSH from GDI stock to draw exactly only an outline?
Re: memory leak fix
On Thu, Jun 26, 2008 at 02:05:15AM -0700, Joris Huizer wrote: Untested, but this seems trivial/obvious, unless I overlooked something From 4026f68e4404c4b1822eefe0b3419db4f896e055 Mon Sep 17 00:00:00 2001 From: Joris Huizer [EMAIL PROTECTED](none) Date: Thu, 26 Jun 2008 11:07:11 +0200 Subject: [PATCH] memory leak fix --- dlls/gdiplus/font.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 0bb9fc7..9ef8edc 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -378,6 +378,7 @@ GpStatus WINGDIPAPI GdipCreateFontFamilyFromName(GDIPCONST WCHAR *name, ffamily-FamilyName = GdipAlloc(LF_FACESIZE * sizeof (WCHAR)); if (!ffamily-FamilyName) { +GdipFree(ffamily-tmw); GdipFree(ffamily); return OutOfMemory; } That's a good start, but there's more. The function leaks an hfont both on success and in this failure case. Also there should be a ReleaseDC in the failure case. In addition it seems to me that the tmw element may as well be an embedded structure rather than a structure pointer. That'll save having to worry about alloc'ing/free'ing it. Huw. -- Huw Davies [EMAIL PROTECTED]
AppDB patches
I see that these patches have not yet been applied to the AppDB. Is there anything wrong with them? * [AppDB] Assorted spelling fixes. http://www.winehq.org/pipermail/wine-patches/2008-June/056192.html * [AppDB] application_queue: Fix update(). http://www.winehq.org/pipermail/wine-patches/2008-June/056193.html -- Francois Gouget [EMAIL PROTECTED] http://fgouget.free.fr/ The greatest programming project of all took six days; on the seventh day the programmer rested. We've been trying to debug the *^%$#@ thing ever since. Moral: design before you implement.
Re: Whats wrong with my patch.
On Thu, Jun 26, 2008 at 3:28 AM, Przemysław Białek [EMAIL PROTECTED] wrote: Hi! I sent a patch last Friday to the wine-patches list. I think that I fulfill the rules that I read on winehq website. Patch is simple implementation of function GdipDrawEllipse, and is based on existing function GdipFillEllipse. This is my first patch to the Wine project. I don't have deeper Wine, and Windows knowledge, so I ask, what else I must do, to get this patch accepted. The link to the mail is below: http://www.winehq.org/pipermail/wine-patches/2008-June/056223.html -- Best Regards Przemysław Białek Your name isn't correct in the patch. Also, adding a testcase to prove the behavior is correct would be helpful.
Re: opengl32: work around a crash in glGetString
I don't understand IF the testcase returns NULL without a context (which should do anyways), both in linux and in windows, and what the patch does is tu ensure that, with no crash, what's bad with it ? In AutoCad's installer it's just called WITHOUT a proper opengl context, in windows it's ok (I guess returning NULL), in wine it crashes; even if the simple testcase doesn't, it still returns NULL. It's obvious that the crash may depend on previous gl calls, or on some previous and/or external bug, but it should still return NULL instead of crashing. Max Louis. Lenders ha scritto: As I mentioned before this requires a test case. As i tried to explain , in a simple test glGetString just return null, it will not crash for me, like it does in the program. I remember from the gl log that the program didn't create a WGL context at all. It isn't allowed to make opengl calls because of that Allowed or not, the program does so, and on windows it installs just fine. Perhaps without a context (if Windows allows it) you get some GDI software renderer string back or so. It just returns (null) Anyway at this point it sounds like a workaround instead of a fix for the real bug. Well, the bug might be outside wine, so then it would be workaround, yes, but still necessary IMO. I cannot see how i can make a testcase based on the output of autocad here: http://bugs.winehq.org/attachment.cgi?id=14005 , other then i did already. It's just an endless sequence of 'wglGetIntegerv','wglFinish', and 'wglFlush', and then a crash in glGetString. I cannot make a test program crash like that; the test program just returns null, like i said before A last stupid question for stefan: how can i make X load the mesa driver, instead of my nvidia? Not happy with your email address? Get the one you really want http://uk.docs.yahoo.com/ymail/new.html - millions of new email addresses available now at Yahoo! http://uk.docs.yahoo.com/ymail/new.html
Re: inetmib1(3/31): Add tests for SnmpExtensionInit
These tests import snmpapi, which isn't available on win95. Do we need to cater for that and do LoadLibrary/GetProcAddress ? Hm. Yeah, I suppose so. That or duplicate the SnmpUtil functions the tests use, which shouldn't be that hard either. --Juan
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
Kai Blin wrote: This fixes a Valgrind warning triggered by the NetApiBufferFree() test where a buffer is free()d twice. @@ -52,6 +52,7 @@ NET_API_STATUS WINAPI NetApiBufferFree(LPVOID Buffer) { TRACE((%p)\n, Buffer); HeapFree(GetProcessHeap(), 0, Buffer); +Buffer = NULL; return NERR_Success; } I don't get it. How does setting a local variable to NULL avoid a warning about freeing it twice? I'm not saying the patch is wrong, I'm just saying I don't get how Valgrind is tricked by this. --Juan
Re: inetmib1(3/31): Add tests for SnmpExtensionInit
Juan Lang [EMAIL PROTECTED] writes: These tests import snmpapi, which isn't available on win95. Do we need to cater for that and do LoadLibrary/GetProcAddress ? Hm. Yeah, I suppose so. That or duplicate the SnmpUtil functions the tests use, which shouldn't be that hard either. Is there really a Win95 setup that has inetmib1 but not snmpapi? -- Alexandre Julliard [EMAIL PROTECTED]
Re: inetmib1(3/31): Add tests for SnmpExtensionInit
Alexandre Julliard wrote: Juan Lang [EMAIL PROTECTED] writes: These tests import snmpapi, which isn't available on win95. Do we need to cater for that and do LoadLibrary/GetProcAddress ? Hm. Yeah, I suppose so. That or duplicate the SnmpUtil functions the tests use, which shouldn't be that hard either. Is there really a Win95 setup that has inetmib1 but not snmpapi? Yes, mine :-) and from the looks at http://test.winehq.org/data/92c8cac214715633459227b2d47df61a18dc8d82/ so does Francois' box. -- Cheers, Paul.
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
On Thursday 26 June 2008 16:34:51 you wrote: @@ -52,6 +52,7 @@ NET_API_STATUS WINAPI NetApiBufferFree(LPVOID Buffer) { TRACE((%p)\n, Buffer); HeapFree(GetProcessHeap(), 0, Buffer); +Buffer = NULL; return NERR_Success; } I don't get it. How does setting a local variable to NULL avoid a warning about freeing it twice? I'm not saying the patch is wrong, I'm just saying I don't get how Valgrind is tricked by this. The trick is the test code. From dlls/netapi32/tests/apibuf.c, lines 44-58 /* test normal logic */ ok(pNetApiBufferAllocate(1024, (LPVOID *)p) == NERR_Success, Reserved memory\n); ok(pNetApiBufferSize(p, dwSize) == NERR_Success, Got size\n); ok(dwSize = 1024, The size is correct\n); ok(pNetApiBufferReallocate(p, 1500, (LPVOID *) p) == NERR_Success, Reallocated\n); ok(pNetApiBufferSize(p, dwSize) == NERR_Success, Got size\n); ok(dwSize = 1500, The size is correct\n); ok(pNetApiBufferFree(p) == NERR_Success, Freed\n); /* test errors handling */ ok(pNetApiBufferFree(p) == NERR_Success, Freed\n); -- valgrind warning Which is to be expected. HeapFree on a NULL pointer doesn't hurt, though. As this test seems to work on Windows, that seemed like the quickest fix. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
context.c
Has anyone looked at the changes put on the list for context.c? Just had not heard anything.. I assume it is because I am new and all but still...just trying to help per the request on the wine site =) chris
valgrind regression in gdiplus
Yesterday, there were a bunch of new valgrind warnings in gdiplus, and they're still there today. See http://kegel.com/wine/valgrind/logs-2008-06-26/vg-gdiplus_font.txt For instance, Invalid read of size 4 at GdipCreateFont (font.c:99) by test_createfont (font.c:57) by func_font (font.c:250) by run_test (test.h:449) by main (test.h:498) Address 0x4 is not stack'd, malloc'd or (recently) free'd ... Conditional jump or move depends on uninitialised value(s) at winetest_vok (test.h:272) by winetest_ok (test.h:301) by test_createfont (font.c:61) by func_font (font.c:250) by run_test (test.h:449) by main (test.h:498) Uninitialised value was created by a stack allocation at test_createfont (font.c:43) ... Syscall param write(buf) points to uninitialised byte(s) at (within /lib/ld-2.7.so) by (within /lib/tls/i686/cmov/libc-2.7.so) by _IO_file_xsputn (in /lib/tls/i686/cmov/libc-2.7.so) by (within /lib/tls/i686/cmov/libc-2.7.so) by vfprintf (in /lib/tls/i686/cmov/libc-2.7.so) by winetest_vok (test.h:279) by winetest_ok (test.h:301) by test_createfont (font.c:65) by func_font (font.c:250) by run_test (test.h:449) by main (test.h:498) Address 0x7f21d657 is on thread 1's stack Uninitialised value was created by a stack allocation at test_createfont (font.c:43)
Re: Whats wrong with my patch.
On Thursday 26 June 2008 13:25:15 Nikolay Sivov wrote: Przemysław Białek wrote: Hi! I sent a patch last Friday to the wine-patches list. I think that I fulfill the rules that I read on winehq website. Patch is simple implementation of function GdipDrawEllipse, and is based on existing function GdipFillEllipse. This is my first patch to the Wine project. I don't have deeper Wine, and Windows knowledge, so I ask, what else I must do, to get this patch accepted. The link to the mail is below: http://www.winehq.org/pipermail/wine-patches/2008-June/056223.html Hi, the only question for me is why don't you select NULL_BRUSH from GDI stock to draw exactly only an outline? It's because I don't know about windows and Wine to much. You mean that I must add the following line (after the prepare_dc, function)? SelectObject(graphics-hdc, GetStockObject(NULL_BRUSH)); If You confirm that this is OK, then I send the updated patch to wine-patches. -- Best Regards Przemysław Białek
Re: context.c
[EMAIL PROTECTED] wrote: Has anyone looked at the changes put on the list for context.c? Just had not heard anything.. I assume it is because I am new and all but still...just trying to help per the request on the wine site =) chris Get the Moviefone Toolbar http://toolbar.aol.com/moviefone/download.html?ncid=aolcmp000511. Showtimes, theaters, movie news, more! It helps if you link to the patches you submitted in the pipermail listing (http://winehq.org/pipermail/wine-patches). This way everyone can see what particular patch you're talking about. -Zac
Re: context.c
Has anyone looked at the changes put on the list for context.c? James already replied: diff is your friend. As the site says, send a diff (in git format, or in unified diff format if for some bad reason you're not using git) if you want feedback, it's too hard to read your changes otherwise. --Juan
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
The fix should be *Buffer = NULL; not Buffer = NULL; since p (in the tests) would still be valid after the NetApiBufferFree call. That's also not correct, as you'd be dereferencing an invalid pointer. NetApiBufferFree takes a void *, not a void ** (unlike NetApiBufferAllocate). My question remains: why does setting a local variable to NULL silence a Valgrind warning? That sounds like a Valgrind bug to me. --Juan
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
On Thu, Jun 26, 2008 at 10:50 AM, Juan Lang [EMAIL PROTECTED] wrote: The fix should be *Buffer = NULL; not Buffer = NULL; since p (in the tests) would still be valid after the NetApiBufferFree call. That's also not correct, as you'd be dereferencing an invalid pointer. NetApiBufferFree takes a void *, not a void ** (unlike NetApiBufferAllocate). My question remains: why does setting a local variable to NULL silence a Valgrind warning? That sounds like a Valgrind bug to me. Not that I know anything but it seems that HeapFree calls RtlFreeHeap and RtlFreeHeap checks to make sure the pointer is not null before trying to free it. So explicitly setting it to null makes the erorr go away. Now trying to free a pointer a second time could maybe be considered poor logic but I'll leave that up to wiser minds. Best, --John Klehm
Re: AppDB patches
On Thu, Jun 26, 2008 at 8:13 AM, Francois Gouget [EMAIL PROTECTED] wrote: I see that these patches have not yet been applied to the AppDB. Is there anything wrong with them? * [AppDB] Assorted spelling fixes. http://www.winehq.org/pipermail/wine-patches/2008-June/056192.html * [AppDB] application_queue: Fix update(). http://www.winehq.org/pipermail/wine-patches/2008-June/056193.html -- Francois Gouget [EMAIL PROTECTED] http://fgouget.free.fr/ The greatest programming project of all took six days; on the seventh day the programmer rested. We've been trying to debug the *^%$#@ thing ever since. Moral: design before you implement. Sorry about missing those patches. Its sometimes difficult to separate the normal patch mail for wine from the patch mail for the appdb. The ratio between the two is quite large. I applied these patches using 'patch' as they weren't in mbox format and wouldn't apply with git-am. I'm not a git expert so I'm not sure if this was the best way to apply them but if I've made a mistake please let me know. Chris
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
Not that I know anything but it seems that HeapFree calls RtlFreeHeap and RtlFreeHeap checks to make sure the pointer is not null before trying to free it. So explicitly setting it to null makes the erorr go away. That might be true if setting Buffer to NULL happened before the call to HeapFree, but it happens after. After the return from NetApiBufferFree, Buffer goes out of scope, and a subsequent call won't be affected by it. So again, this seems like a Valgrind bug. How is the warning getting silenced by this assignment? --Juan
Re: [2/2] ntdll: rtlstr.c: Implement checking for control characters in RtlIsTextUnicode [try 7]
Zac Brown wrote: Add checking for control characters in both standard and byte-reversed forms to (Rtl)IsTextUnicode. Changelog: * Add if-statement to ensure we only found control characters in the first 256 indices. If we found them beyond that, we don't care since Windows doesn't either. --- dlls/ntdll/rtlstr.c | 23 +++ dlls/ntdll/tests/rtlstr.c | 21 + 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/dlls/ntdll/rtlstr.c b/dlls/ntdll/rtlstr.c index deec931..802782f 100644 --- a/dlls/ntdll/rtlstr.c +++ b/dlls/ntdll/rtlstr.c @@ -1591,7 +1591,10 @@ NTSTATUS WINAPI RtlFindCharInUnicodeString( */ BOOLEAN WINAPI RtlIsTextUnicode( LPCVOID buf, INT len, INT *pf ) { +static const WCHAR std_control_chars[] = {'\r','\n','\t',' ',0x3000,0}; +static const WCHAR byterev_control_chars[] = {0x0d00,0x0a00,0x0900,0x2000,0x0030,0}; const WCHAR *s = buf; +WCHAR *tmp_ptr; int i; unsigned int flags = ~0U, out_flags = 0; @@ -1650,6 +1653,26 @@ BOOLEAN WINAPI RtlIsTextUnicode( LPCVOID buf, INT len, INT *pf ) } } +/* These two tests are not mutually exclusive so regardless of whether */ +/* the string has the BOM for byte-reversed order, it still checks */ +/* the string if the flag is set. */ + +/* Check for standard Unicode control characters. */ +if (flags IS_TEXT_UNICODE_CONTROLS) +{ +tmp_ptr = strpbrkW(s, std_control_chars); +if (tmp_ptr != NULL (tmp_ptr - s) len) +out_flags |= IS_TEXT_UNICODE_CONTROLS; +} + +/* Check for byte-reversed Unicode control characters. */ +if (flags IS_TEXT_UNICODE_REVERSE_CONTROLS strpbrkW(s, byterev_control_chars) != NULL) +{ +tmp_ptr = strpbrkW(s, byterev_control_chars); +if (tmp_ptr != NULL (tmp_ptr - s) len) +out_flags |= IS_TEXT_UNICODE_REVERSE_CONTROLS; +} + if (pf) { out_flags = *pf; diff --git a/dlls/ntdll/tests/rtlstr.c b/dlls/ntdll/tests/rtlstr.c index 74ba8ea..d5d8839 100644 --- a/dlls/ntdll/tests/rtlstr.c +++ b/dlls/ntdll/tests/rtlstr.c @@ -1693,7 +1693,6 @@ static void test_RtlIsTextUnicode(void) flags = IS_TEXT_UNICODE_UNICODE_MASK; ok(pRtlIsTextUnicode(unicode, sizeof(unicode), flags), Text should not pass a Unicode\n); -todo_wine ok(flags == (IS_TEXT_UNICODE_STATISTICS | IS_TEXT_UNICODE_CONTROLS), Expected flags 0x6, obtained %x\n, flags); @@ -1712,7 +1711,7 @@ static void test_RtlIsTextUnicode(void) be_unicode[i + 1] = (unicode[i] 8) | ((unicode[i] 0xff) 8); } ok(!pRtlIsTextUnicode(be_unicode, sizeof(unicode) + 2, NULL), Reverse endian should not be Unicode\n); -todo_wine ok(!pRtlIsTextUnicode(be_unicode[1], sizeof(unicode), NULL), Reverse endian should not be Unicode\n); +ok(!pRtlIsTextUnicode(be_unicode[1], sizeof(unicode), NULL), Reverse endian should not be Unicode\n); flags = IS_TEXT_UNICODE_REVERSE_MASK; ok(!pRtlIsTextUnicode(be_unicode[1], sizeof(unicode), flags), Reverse endian should be Unicode\n); @@ -1722,7 +1721,6 @@ static void test_RtlIsTextUnicode(void) flags = IS_TEXT_UNICODE_REVERSE_MASK; ok(!pRtlIsTextUnicode(be_unicode, sizeof(unicode) + 2, flags), Reverse endian should be Unicode\n); -todo_wine ok(flags == (IS_TEXT_UNICODE_REVERSE_CONTROLS | IS_TEXT_UNICODE_REVERSE_SIGNATURE), Expected flags 0xc0, obtained %x\n, flags); @@ -1750,11 +1748,8 @@ static void test_RtlIsTextUnicode(void) ok(flags == 0, Expected flags 0x0, obtained %x\n, flags); flags = IS_TEXT_UNICODE_CONTROLS; -todo_wine -{ -ok(pRtlIsTextUnicode(unicode, sizeof(unicode), flags), Test should pass on Unicode string lacking control characters.\n); -ok(flags == IS_TEXT_UNICODE_CONTROLS, Expected flags 0x04, obtained %x\n, flags); -} +ok(pRtlIsTextUnicode(unicode, sizeof(unicode), flags), Test should pass on Unicode string lacking control characters.\n); +ok(flags == IS_TEXT_UNICODE_CONTROLS, Expected flags 0x04, obtained %x\n, flags); flags = IS_TEXT_UNICODE_CONTROLS; ok(!pRtlIsTextUnicode(be_unicode_no_controls, sizeof(unicode_no_controls) + 2, flags), @@ -1762,11 +1757,8 @@ static void test_RtlIsTextUnicode(void) ok(flags == 0, Expected flags 0x0, obtained %x\n, flags); flags = IS_TEXT_UNICODE_CONTROLS; -todo_wine -{ -ok(pRtlIsTextUnicode(mixed_controls, sizeof(mixed_controls), flags), Test should pass on a string containing control characters.\n); -ok(flags == IS_TEXT_UNICODE_CONTROLS, Expected flags 0x04, obtained %x\n, flags); -} +ok(pRtlIsTextUnicode(mixed_controls, sizeof(mixed_controls), flags), Test should pass on a string
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
On Thursday 26 June 2008 18:07:59 you wrote: Not that I know anything but it seems that HeapFree calls RtlFreeHeap and RtlFreeHeap checks to make sure the pointer is not null before trying to free it. So explicitly setting it to null makes the erorr go away. That might be true if setting Buffer to NULL happened before the call to HeapFree, but it happens after. After the return from NetApiBufferFree, Buffer goes out of scope, and a subsequent call won't be affected by it. You said Buffer was a local variable, but it's a parameter passed from the test. So, as far as I can see, the test does Buffer = HeapAlloc(GetProcessHeap(), NULL, 1024); HeapReAlloc(GetProcessHeap(), 0, Buffer, 1500); HeapFree(GetProcessHeap(), 0, Buffer); HeapFree(GetProcessHeap(), 0, Buffer); That's a double free, right? Valgrind is complaining about the second one. HeapFree() on a NULL buffer doesn't do anything, so it's not causing a Valgrind error anymore. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
You said Buffer was a local variable, but it's a parameter passed from the test. From the point of view of NetApiBufferFree, Buffer is a parameter, which is equivalent to a local variable. In C, all parameters are pass-by-value. You can simulate pass-by-reference by passing a pointer, but it's still pass by (pointer) value. So, as far as I can see, the test does Buffer = HeapAlloc(GetProcessHeap(), NULL, 1024); HeapReAlloc(GetProcessHeap(), 0, Buffer, 1500); HeapFree(GetProcessHeap(), 0, Buffer); HeapFree(GetProcessHeap(), 0, Buffer); That's a double free, right? Valgrind is complaining about the second one. Sort of. I'll get back to your example in a second. Valgrind rightly complains about the second, double free. I think that's what the test was aiming to check: whether a double free resulted in NERR_Success being returned. Whether that's a valid thing to check might be debatable, but it does so, and I think the Valgrind warning is appropriate. My question is, why does assigning Buffer to NULL in the context of NetApiBufferFree result in the warning going away? Remember, the first instance Buffer is different from the second. If we label the first stack frame with ' and the second with '', what we really have is: Buffer = HeapAlloc(GetProcessHeap(), NULL, 1024); HeapReAlloc(GetProcessHeap(), 0, Buffer, 1500); Buffer' = Buffer; /* A new copy of Buffer is created in the context of NetApiBufferFree */ HeapFree(GetProcessHeap(), 0, Buffer'); Buffer'' = Buffer; /* A second copy of Buffer is created in the context of NetApiBufferFree */ HeapFree(GetProcessHeap(), 0, Buffer''); So setting Buffer' to NULL has no impact on the HeapFree of Buffer''. The fact that this makes a Valgrind warning disappear still seems like a Valgrind bug. If it were doing static analysis, I'd say it's gotten confused by aliasing. Since it's doing dynamic analysis, I have to ask why that's causing the warning to go away. --Juan
Re: netapi32: Set the buffer to NULL in NetApiBufferFree
On Thursday 26 June 2008 19:51:41 Juan Lang wrote: So setting Buffer' to NULL has no impact on the HeapFree of Buffer''. The fact that this makes a Valgrind warning disappear still seems like a Valgrind bug. If it were doing static analysis, I'd say it's gotten confused by aliasing. Since it's doing dynamic analysis, I have to ask why that's causing the warning to go away. I didn't test it. Thinking about this a bit more, you're obviously correct. I shouldn't write patches before 8am, I guess. My fault, never mind. Alexandre suggested just killing the second NetApiBufferFree() call, so that's what I'm going to do. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: context.c diff file...
On Thu, Jun 26, 2008 at 02:06:18PM -0400, [EMAIL PROTECTED] wrote: ok sorry for cutting and pasting the whole routine.. did not see to do just the diff ... (give the old guy a break.. I am an architect now not a programmer G) Please pass at least the -u option for diff. Ciao, Marcus
ok lets try this yet again
I am sorry everyone for the spam =) been awhile since I developed stuff, been so used to designing things? =) here in all its glorry is the diff -u context.c context.c.new diff.txt file I couldnt get the -c option to work so here is just the -u --- context.c??? 2008-06-26 13:52:57.0 -0400 +++ context.c.change??? 2008-06-26 14:32:48.0 -0400 @@ -116,13 +116,12 @@ int iPixelFormat=0; short redBits, greenBits, blueBits, alphaBits, colorBits; short depthBits=0, stencilBits=0; - int i = 0; int nCfgs = This-adapter-nCfgs; -??? WineD3D_PixelFormat *cfgs = This-adapter-cfgs; ? -??? TRACE(ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d, numSamples=%d, pbuffer=%d, findCompatible=%d\n, -? debug_d3dformat(ColorFormat), debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer, findCompatible); +??? WineD3D_PixelFormat *cfgs = This-adapter-cfgs; +??? BOOL exactDepthMatch = FALSE;? /*Changed june 23,08 */?? +??? PIXELFORMATDESCRIPTOR pfd; /*Changed june 23,08 */ ? if(!getColorBits(ColorFormat, redBits, greenBits, blueBits, alphaBits, colorBits)) { ERR(Unable to get color bits for format %s (%#x)!\n, debug_d3dformat(ColorFormat), ColorFormat); @@ -145,84 +144,91 @@ ? DepthStencilFormat = WINED3DFMT_D24S8; ? -??? if(DepthStencilFormat) { -??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); -??? } +/* Changed Section June 24,08 */ + +#if 0 +??? if(DepthStencilFormat) +? { +??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); +??? } +#endif + +/* Just call getDepthStencilBits as the above IF will always in this case be true */ + +??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); ? /* Find a pixel format which EXACTLY matches our requirements (except for depth) */ -??? for(i=0; inCfgs; i++) { -??? BOOL exactDepthMatch = TRUE; +??? for(i=0; inCfgs; i++) +? { cfgs = This-adapter-cfgs[i]; - +??? /* For now only accept RGBA formats. Perhaps some day we will ? * allow floating point formats for pbuffers. */ if(cfgs-iPixelType != WGL_TYPE_RGBA_ARB) -??? continue; +?? continue; ? /* In window mode (!pbuffer) we need a window drawable format and double buffering. */ if(!pbuffer !(cfgs-windowDrawable cfgs-doubleBuffer)) -??? continue; +?? continue; ? -??? /* We like to have aux buffers in backbuffer mode */ +?? /* We like to have aux buffers in backbuffer mode */ if(auxBuffers !cfgs-auxBuffers) -??? continue; +?? continue; ? /* In pbuffer-mode we need a pbuffer-capable format but we don't want double buffering */ if(pbuffer (!cfgs-pbufferDrawable || cfgs-doubleBuffer)) -??? continue; + continue; ? -??? if(cfgs-redSize != redBits) -??? continue; -??? if(cfgs-greenSize != greenBits) -??? continue; -??? if(cfgs-blueSize != blueBits) -??? continue; -??? if(cfgs-alphaSize != alphaBits) -??? continue; - -??? /* We try to locate a format which matches our requirements exactly. In case of - * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */ -??? if(cfgs-depthSize depthBits) -??? continue; -??? else if(cfgs-depthSize depthBits) -??? exactDepthMatch = FALSE; +??? if ((cfgs-redSize != redBits) || (cfgs-greenSize != greenBits) || (cfgs-blueSize != blueBits) || (cfgs-alphaSize != alphaBits)) + continue; ? /* In all cases make sure the number of stencil bits matches our requirements ? * even when we don't need stencil because it could affect performance EXCEPT ? * on cards which don't offer depth formats without stencil like the i915 drivers ? * on Linux. */ -??? if(stencilBits != cfgs-stencilSize !(This-adapter-brokenStencil stencilBits = cfgs-stencilSize)) -??? continue; +??? if((stencilBits != cfgs-stencilSize) !((This-adapter-brokenStencil stencilBits) = cfgs-stencilSize)) + continue; ? /* Check multisampling support */ if(cfgs-numSamples != numSamples) -??? continue; + continue; ? -??? /* When we have passed all the checks then we have found a format which matches our - * requirements. Note that we only check for a limit number of capabilities right now, - * so there can easily be a dozen of pixel formats which appear to be the 'same' but - * can still differ in things like multisampling, stereo, SRGB and other flags. - */ +??? /* We try to locate a format which matches our requirements exactly. In case of + * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */ + if (cfgs-depthSize !=? depthBits) +??? continue; ? /* Exit the loop
Re: context.c diff file...
On Thu, Jun 26, 2008 at 02:06:18PM -0400, [EMAIL PROTECTED] wrote: ok sorry for cutting and pasting the whole routine.. did not see to do just the diff ... (give the old guy a break.. I am an architect now not a programmer G) Your diff is getting corrupted. See all the ???: http://www.winehq.org/pipermail/wine-devel/2008-June/066714.html Also, don't send patches in HTML (your message is multipart, one part corrupted and one HTML). I suspect you can fix both these problems by using a different MUA (i.e., not AOL), and not copy/pasting a patch if that is what you are doing. The best thing in your cases is probably to follow the directions here for generating a patch: http://wiki.winehq.org/GitWine and then send the patch as an attachment. Your patch should be a unified diff (diff -u), which it should be by default if you follow the instructions above. WRT the patch content itself, there is a lot of whitespace-only change. The patch should be clear in its purpose. Whitespace-only changes detract from the readability of the patch. Lastly, once the patch is clean and you can send it correctly (test on yourself and compare with patches sent that get accepted), send it to the correct list for patches, which is wine-patches, not wine-devel. Good luck.
Re: ok lets try this yet again
On Thu, Jun 26, 2008 at 1:38 PM, [EMAIL PROTECTED] wrote: I am sorry everyone for the spam =) been awhile since I developed stuff, been so used to designing things =) here in all its glorry is the diff -u context.c context.c.new diff.txt file I couldnt get the -c option to work so here is just the -u --- context.c2008-06-26 13:52:57.0 -0400 +++ context.c.change2008-06-26 14:32:48.0 -0400 @@ -116,13 +116,12 @@ int iPixelFormat=0; short redBits, greenBits, blueBits, alphaBits, colorBits; short depthBits=0, stencilBits=0; - int i = 0; int nCfgs = This-adapter-nCfgs; -WineD3D_PixelFormat *cfgs = This-adapter-cfgs; -TRACE(ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d, numSamples=%d, pbuffer=%d, findCompatible=%d\n, - debug_d3dformat(ColorFormat), debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer, findCompatible); +WineD3D_PixelFormat *cfgs = This-adapter-cfgs; +BOOL exactDepthMatch = FALSE; /*Changed june 23,08 */ +PIXELFORMATDESCRIPTOR pfd; /*Changed june 23,08 */ if(!getColorBits(ColorFormat, redBits, greenBits, blueBits, alphaBits, colorBits)) { ERR(Unable to get color bits for format %s (%#x)!\n, debug_d3dformat(ColorFormat), ColorFormat); @@ -145,84 +144,91 @@ DepthStencilFormat = WINED3DFMT_D24S8; -if(DepthStencilFormat) { -getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); -} +/* Changed Section June 24,08 */ + +#if 0 +if(DepthStencilFormat) + { +getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); +} +#endif + +/* Just call getDepthStencilBits as the above IF will always in this case be true */ + +getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); /* Find a pixel format which EXACTLY matches our requirements (except for depth) */ -for(i=0; inCfgs; i++) { -BOOL exactDepthMatch = TRUE; +for(i=0; inCfgs; i++) + { cfgs = This-adapter-cfgs[i]; - + /* For now only accept RGBA formats. Perhaps some day we will * allow floating point formats for pbuffers. */ if(cfgs-iPixelType != WGL_TYPE_RGBA_ARB) -continue; + continue; /* In window mode (!pbuffer) we need a window drawable format and double buffering. */ if(!pbuffer !(cfgs-windowDrawable cfgs-doubleBuffer)) -continue; + continue; -/* We like to have aux buffers in backbuffer mode */ + /* We like to have aux buffers in backbuffer mode */ if(auxBuffers !cfgs-auxBuffers) -continue; + continue; /* In pbuffer-mode we need a pbuffer-capable format but we don't want double buffering */ if(pbuffer (!cfgs-pbufferDrawable || cfgs-doubleBuffer)) -continue; + continue; -if(cfgs-redSize != redBits) -continue; -if(cfgs-greenSize != greenBits) -continue; -if(cfgs-blueSize != blueBits) -continue; -if(cfgs-alphaSize != alphaBits) -continue; - -/* We try to locate a format which matches our requirements exactly. In case of - * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */ -if(cfgs-depthSize depthBits) -continue; -else if(cfgs-depthSize depthBits) -exactDepthMatch = FALSE; +if ((cfgs-redSize != redBits) || (cfgs-greenSize != greenBits) || (cfgs-blueSize != blueBits) || (cfgs-alphaSize != alphaBits)) + continue; /* In all cases make sure the number of stencil bits matches our requirements * even when we don't need stencil because it could affect performance EXCEPT * on cards which don't offer depth formats without stencil like the i915 drivers * on Linux. */ -if(stencilBits != cfgs-stencilSize !(This-adapter-brokenStencil stencilBits = cfgs-stencilSize)) -continue; +if((stencilBits != cfgs-stencilSize) !((This-adapter-brokenStencil stencilBits) = cfgs-stencilSize)) + continue; /* Check multisampling support */ if(cfgs-numSamples != numSamples) -continue; + continue; -/* When we have passed all the checks then we have found a format which matches our - * requirements. Note that we only check for a limit number of capabilities right now, - * so there can easily be a dozen of pixel formats which appear to be the 'same' but - * can still differ in things like multisampling, stereo, SRGB and other flags. - */ +/* We try to locate a format which matches our requirements exactly. In case of + * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept
Re: context.c diff file...
Hi Dan, most of that advice was good, but I'll comment on one thing you said: Lastly, once the patch is clean and you can send it correctly (test on yourself and compare with patches sent that get accepted), send it to the correct list for patches, which is wine-patches, not wine-devel. It's fairly normal to invite comment on a patch on wine-devel before sending it to wine-patches. I think that's fine, especially as he's just starting out. --Juan
Re: [6/7] gdiplus: Implemented GdipGetPathData with test
Hi Nikolay: +pathData-Points = GdipAlloc(sizeof(PointF) * pathData-Count); +if(!pathData-Points) +return OutOfMemory; + +pathData-Types = GdipAlloc(pathData-Count); +if(!pathData-Points) Greetings from Copy Paste pathData-Points = pathData-Types +return OutOfMemory; You are leaking the memory in pathData-Points -- By by ... Detlef
Re: quartz Rewrite the avi splitter so that video and sound run in separate threads, with tests
On Mi, 2008-06-25 at 21:34 -0700, Maarten Lankhorst wrote: Because of the magnitude of the change I cannot split this patc hup, my apologies for this. Without any view at your code, the sentense above is wrong by design. A Test is always independant from the implementation, and what permit a seperate patch for test_threads() to split the tests? -- By by ... Detlef
Re: ok lets try this yet again
As others have said, there are plenty of unnecessary changes, but you probably need some pointing at them, so here goes: --- context.c2008-06-26 13:52:57.0 -0400 +++ context.c.change2008-06-26 14:32:48.0 -0400 @@ -116,13 +116,12 @@ int iPixelFormat=0; short redBits, greenBits, blueBits, alphaBits, colorBits; short depthBits=0, stencilBits=0; - Unneeded. int i = 0; int nCfgs = This-adapter-nCfgs; -WineD3D_PixelFormat *cfgs = This-adapter-cfgs; -TRACE(ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d, numSamples=%d, pbuffer=%d, findCompatible=%d\n, - debug_d3dformat(ColorFormat), debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer, findCompatible); Why did you remove the trace? +WineD3D_PixelFormat *cfgs = This-adapter-cfgs; +BOOL exactDepthMatch = FALSE; /*Changed june 23,08 */ +PIXELFORMATDESCRIPTOR pfd; /*Changed june 23,08 */ Don't put comments on the change date in the code, git history will tell us all we need to know about that. if(!getColorBits(ColorFormat, redBits, greenBits, blueBits, alphaBits, colorBits)) { ERR(Unable to get color bits for format %s (%#x)!\n, debug_d3dformat(ColorFormat), ColorFormat); @@ -145,84 +144,91 @@ DepthStencilFormat = WINED3DFMT_D24S8; -if(DepthStencilFormat) { -getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); -} +/* Changed Section June 24,08 */ + +#if 0 +if(DepthStencilFormat) + { +getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); +} +#endif + +/* Just call getDepthStencilBits as the above IF will always in this case be true */ + +getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); While you're right that this if is unnecessary, 1) putting a redundant copy if an #if 0 block is really wasteful, as it doesn't give future developers any hint as to when, if ever, the block should be removed. 2) The comment is also unnecessary, as the above IF won't refer to anything once it's removed. Just remove the redundant if and be done with it. Also, this change is really a no-op, since the if is guaranteed to be true. As such it should probably be a separate patch from any other change. /* Find a pixel format which EXACTLY matches our requirements (except for depth) */ -for(i=0; inCfgs; i++) { -BOOL exactDepthMatch = TRUE; +for(i=0; inCfgs; i++) + { Why did you re-indent the brace? Just leave it where it is, please. cfgs = This-adapter-cfgs[i]; - + Here you've added a bunch of white space. Please don't do that. /* For now only accept RGBA formats. Perhaps some day we will * allow floating point formats for pbuffers. */ if(cfgs-iPixelType != WGL_TYPE_RGBA_ARB) -continue; + continue; Another whitespace-only change. /* In window mode (!pbuffer) we need a window drawable format and double buffering. */ if(!pbuffer !(cfgs-windowDrawable cfgs-doubleBuffer)) -continue; + continue; and again.. -/* We like to have aux buffers in backbuffer mode */ + /* We like to have aux buffers in backbuffer mode */ and again.. if(auxBuffers !cfgs-auxBuffers) -continue; + continue; and again.. getting the picture? /* In pbuffer-mode we need a pbuffer-capable format but we don't want double buffering */ if(pbuffer (!cfgs-pbufferDrawable || cfgs-doubleBuffer)) -continue; + continue; more.. -if(cfgs-redSize != redBits) -continue; -if(cfgs-greenSize != greenBits) -continue; -if(cfgs-blueSize != blueBits) -continue; -if(cfgs-alphaSize != alphaBits) -continue; - -/* We try to locate a format which matches our requirements exactly. In case of - * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */ -if(cfgs-depthSize depthBits) -continue; -else if(cfgs-depthSize depthBits) -exactDepthMatch = FALSE; +if ((cfgs-redSize != redBits) || (cfgs-greenSize != greenBits) || (cfgs-blueSize != blueBits) || (cfgs-alphaSize != alphaBits)) + continue; /* In all cases make sure the number of stencil bits matches our requirements * even when we don't need stencil because it could affect performance EXCEPT * on cards which don't offer depth formats without stencil like the i915 drivers * on Linux. */ -if(stencilBits != cfgs-stencilSize !(This-adapter-brokenStencil stencilBits = cfgs-stencilSize)) -continue; +if((stencilBits != cfgs-stencilSize) !((This-adapter-brokenStencil stencilBits) = cfgs-stencilSize)) + continue; more..
Re: gdiplus: font.c: ensure to release resources
Joris Huizer wrote: A modified patch, taking hints into account from Huw Davies You need to select out the hfont before deleting it. So the SelectObject should look like hfont_old = SelectObject(hdc, hfont); then after the GetTextMetrics do DeleteObject(SelectObject(hdc, hfont_old)) (then you don't need the two sets of DeleteObject) Huw.
Re: ok lets try this yet again
On Do, 2008-06-26 at 14:38 -0400, [EMAIL PROTECTED] wrote: I have no Idea about the touched code, but still some more hints: You must use your real name +WineD3D_PixelFormat *cfgs = This-adapter-cfgs; +BOOL exactDepthMatch = FALSE; /*Changed june 23,08 */ +PIXELFORMATDESCRIPTOR pfd; /*Changed june 23,08 */ Why do you never see similar comments in any Open Source Project? - It's useless in the source - That and more informations are available in the software control system (wine uses git) +/* Changed Section June 24,08 */ + See Above +#if 0 +if(DepthStencilFormat) + { +getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); +} +#endif You add unused sourcecode here. Why? Another example, what you missed is the indention in Wine: The same as in the rest of the File. Default is 4 Space Please read our Wiki ( http://wiki.winehq.org/Developers ), before sending patches. Thanks for helping Wine -- By by ... Detlef
Ok guys...
Thank you so much for being patient and showing me how to do this.. with much help from everyone here is what the results are : Patch 1 : File : wine-1.0/dlls/wined3d/context.c --- context.c??? 2008-06-26 13:52:57.0 -0400 +++ context.c.patch1??? 2008-06-26 15:19:15.0 -0400 @@ -145,9 +145,7 @@ static int WineD3D_ChoosePixelFormat(IWi ? DepthStencilFormat = WINED3DFMT_D24S8; ? -??? if(DepthStencilFormat) { -??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); -??? } +??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); ? /* Find a pixel format which EXACTLY matches our requirements (except for depth) */ for(i=0; inCfgs; i++) { Patch 2: File: wine-1.0/dlls/wined3d/context.c --- context.c??? 2008-06-26 13:52:57.0 -0400 +++ context.c.patch2??? 2008-06-26 15:37:25.0 -0400 @@ -120,7 +120,9 @@ static int WineD3D_ChoosePixelFormat(IWi int i = 0; int nCfgs = This-adapter-nCfgs; WineD3D_PixelFormat *cfgs = This-adapter-cfgs; - +??? PIXELFORMATDESCRIPTOR pfd; +??? BOOL exactDepthMatch = TRUE; +??? TRACE(ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d, numSamples=%d, pbuffer=%d, findCompatible=%d\n, ?? debug_d3dformat(ColorFormat), debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer, findCompatible); ? @@ -151,7 +153,6 @@ static int WineD3D_ChoosePixelFormat(IWi ? /* Find a pixel format which EXACTLY matches our requirements (except for depth) */ for(i=0; inCfgs; i++) { -??? BOOL exactDepthMatch = TRUE; cfgs = This-adapter-cfgs[i]; ? /* For now only accept RGBA formats. Perhaps some day we will @@ -180,13 +181,7 @@ static int WineD3D_ChoosePixelFormat(IWi if(cfgs-alphaSize != alphaBits) continue; ? -??? /* We try to locate a format which matches our requirements exactly. In case of - * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */ -??? if(cfgs-depthSize depthBits) -??? continue; -??? else if(cfgs-depthSize depthBits) -??? exactDepthMatch = FALSE; - +??? /* In all cases make sure the number of stencil bits matches our requirements ? * even when we don't need stencil because it could affect performance EXCEPT ? * on cards which don't offer depth formats without stencil like the i915 drivers @@ -198,6 +193,13 @@ static int WineD3D_ChoosePixelFormat(IWi if(cfgs-numSamples != numSamples) continue; ? +?? /* We try to locate a format which matches our requirements exactly. In case of + * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */ +??? if(cfgs-depthSize depthBits) +??? continue; +??? else if(cfgs-depthSize depthBits) +??? exactDepthMatch = FALSE; + /* When we have passed all the checks then we have found a format which matches our ? * requirements. Note that we only check for a limit number of capabilities right now, ? * so there can easily be a dozen of pixel formats which appear to be the 'same' but @@ -216,12 +218,8 @@ static int WineD3D_ChoosePixelFormat(IWi } } ? -??? /* When findCompatible is set and no suitable format was found, let ChoosePixelFormat choose a pixel format in order not to crash. */ -??? if(!iPixelFormat !findCompatible) { -??? ERR(Can't find a suitable iPixelFormat\n); -??? return FALSE; -??? } else if(!iPixelFormat) { -??? PIXELFORMATDESCRIPTOR pfd; +??? if(!iPixelFormat) { + ? TRACE(Falling back to ChoosePixelFormat as we weren't able to find an exactly matching pixel format\n); /* PixelFormat selection */ Chris Ahrendt P.S. THANKS everyone.. so now comments please? G
re: Ok guys...
Your subject line was confusing; what's this about? Your patches seem malformed, they come across with question marks for tabs, e.g. --- context.c??? 2008-06-26 13:52:57.0 -0400 +++ context.c.patch1??? 2008-06-26 15:19:15.0 -0400 @@ -145,9 +145,7 @@ static int WineD3D_ChoosePixelFormat(IWi ? DepthStencilFormat = WINED3DFMT_D24S8; ? -??? if(DepthStencilFormat) { -??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); -??? } +??? getDepthStencilBits(DepthStencilFormat, depthBits, stencilBits); Perhaps you should resend, using a proper subject line, attaching one patch per message, and providing a good description of what each patch does. If you expect the patches to be applied, they need to go to wine-patches, not wine-devel.
yes I am trying to get feedback before I put it...
on the patch list... The first patch is for a bug I tracked down in context.c that is causing some of the behavior that wine is having with the ATI graphics driver. The second patch? was something I noticed in the code as well that looked a little off which also relates to the way iPixel shaders are init in the d3d code. So I came on here to get some feedback first before taking it to the patch group as something. This was to start getting my feet wet before I took up potentially some development task or the like. not sure about why the mailer is replacing spaces with ? that I do not know... Chris Ahrendt
Re: gdiplus/graphicspath test crashes
On Wed, 2008-06-25 at 10:41 +0200, Paul Vriens wrote: Hi, I've noticed that the mentioned tests crashes when run via Paul Millar's winetest executable. Compiling myself and running the test works fine. Any one else seeing this? I've noticed it crashes if Wine is compiled with -O0, but not when with -O2 (the default in most cases). signature.asc Description: This is a digitally signed message part
Re: yes I am trying to get feedback before I put it...
[EMAIL PROTECTED] wrote: on the patch list... The first patch is for a bug I tracked down in context.c that is causing some of the behavior that wine is having with the ATI graphics driver. That patch is no-op - it doesn't change anything. The second patch was something I noticed in the code as well that looked a little off which also relates to the way iPixel shaders are init in the d3d code. I don't think the second patch is correct. Some parts of it - might be but not all. -/* When findCompatible is set and no suitable format was found, let ChoosePixelFormat choose a pixel format in order not to crash. */ -if(!iPixelFormat !findCompatible) { -ERR(Can't find a suitable iPixelFormat\n); -return FALSE; -} else if(!iPixelFormat) { -PIXELFORMATDESCRIPTOR pfd; +if(!iPixelFormat) { + Here you just ignoring the problem and not fixing anything. If Wine coudn't find compatible format - then something is wrong and it's an error. not sure about why the mailer is replacing spaces with ? that I do not know... First of all use text only (no html) messages. Second, attach your patches as text files to the e-mail message.
Re: msxml3: Added xml Input Callbacks to support loading files from aWine windows drive (try 2)
Alistair Leslie-Hughes [EMAIL PROTECTED] wrote in message news:[EMAIL PROTECTED] +/* Support for loading xml files from a Wine Windows drive */ +static int wineXmlMatchCallback (char const * filename) +{ +BSTR sFilename = bstr_from_xmlChar( (xmlChar*)filename); +BSTR sColon = bstr_from_xmlChar( (xmlChar*):); +int nRet = 0; + +TRACE(%s\n, debugstr_w(sFilename)); + +/* + * We will deal with loading XML files from the file system + * We only care about files that linux cannot find. + *e.g. C:,D: etc + */ +if(iswalpha(sFilename[0]) sFilename[1] == sColon[0]) +nRet = 1; + Hi, The file detection is what is causing this patch not to be accepted. For reference http://bugs.winehq.org/show_bug.cgi?id=11325 - Summary: This script attempts to load a script from disk with a windows path (Z:\blah\rooms\room1.xml), and libxml doesnt understand how to use load this file. During startup, libxml attempts to load its DTD's from the /etc/ directory, (it uses the format file:///etc/xml/DTD.xml), and we dont want to handle these files, but we do want to handle windows paths. What method can I use to detected a windows path verses a unix path? Best Regards Alistair Leslie-Hughes
Re: Ok guys...
On Thu, Jun 26, 2008 at 2:11 PM, Reece Dunn [EMAIL PROTECTED] wrote: 2008/6/26 Dan Kegel [EMAIL PROTECTED]: Your subject line was confusing; what's this about? Celticht32, please don't change the subject line when replying. It makes it very hard to follow the thread. And please use a descriptive subject line; this isn't a forum, it's a mailing list, and the rules are different.
Re: richedit: Implemented EM_STOPGROUPTYPING, fixed undo grouping (with tests)
Alex Villacís Lasso wrote: Dylan Smith escribió: EM_STOPGROUPTYPING simply ends the undo coalescing transaction. The remarks for this message on MSDN explains what events stops group typing, which led me to adding the delete key to the actions that stop group typing. The tests that are included with this patch verify the correctness of the changes. This patch depends on a previous patch I submitted, which has an email subject [6/44] richedit: Implemented undo coalescing to group typing events, and was submitted on June 17. --- dlls/riched20/editor.c | 12 ++-- dlls/riched20/tests/editor.c | 40 2 files changed, 50 insertions(+), 2 deletions(-) This patch has the corresponding test. Could you please send a patch (one or more) that would test the behaviors fixed by the previous patches? This will make it more likely for AJ to accept the patches, and will also prevent someone (such as myself) from accidentally breaking fixed behavior with further changes. Alex: You are talking about fixing things. I was fixing EM_FONTRANGE before 1.0 as it breaks several things. Are you looking to pick this up as one of the things you are going to fix? James McKenzie
Some conformance tests a bit slow under Valgrind
Under Valgrind, on my nice fast e7200 system, the entire suite of tests takes about three hours to run. The slowest ten percent of the tests take over a third of the runtime. riched20's editor.c in particular spends way too long (I think) on test_EM_AUTOURLDETECT. The min, median, and max times to run a test are 10 seconds, 19 seconds, and 15 minutes. 15:44.05elapsed riched20.dll editor.c 6:35.94elapsed d3d9.dll visual.c 5:45.43elapsed shell32.dll shlexec.c 3:28.98elapsed kernel32.dll process.c 2:49.85elapsed winmm.dll wave.c 2:32.18elapsed kernel32.dll debugger.c 2:14.41elapsed gdiplus.dll font.c 1:44.54elapsed dsound.dll capture.c 1:29.51elapsed mshtml.dll dom.c ... 0:18.84elapsed shlwapi.dll path.c 0:18.83elapsed rpcrt4.dll cstub.c ... 0:09.92elapsed ntdll.dll path.c 0:09.92elapsed msvcrt.dll data.c I guess I'm not complaining, but perhaps we could tone down test_EM_AUTOURLDETECT a bit.
Re: README: Add Russian translation (2nd try)
Vladimir Pankratov wrote: Hello. Changelog: Added README.ru file. Corrected some errors and fixed text formatting (as I think) since last patch. Thanks. Further suggestions: Wine это программа которая позволяет запускать программы Microsoft Windows (включая DOS, Windows 3.x и Win32) на Unix. Она состоит из программы загрузки которая загружает и выполняет программы Microsoft Windows,и библиотеки (Winelib) которая реализует вызовы Windows API используя их Unix или X11 эквиваленты. Библиотека также может быть использована для переноса кода Win32 в исполняемые файлы Unix. Lots of punctuation here. Such as: - программа, которая - программы загрузки, которая - библиотеки (Winelib), которая It's better to say: портирования кода Win32 в среду Unix. Каждый раз когда вы компилируете исходный код, рекомендуется использовать Wine Installer для компоновки и установки Wine. The same: - Каждый раз, когда Для того чтобы скомпилировать и запустить Wine, необходимо одно из следующих: - Компиляция и запуск Wine поддерживается в следующих операционных системах: Linux версии 2.0.36 или выше FreeBSD 7.0 или позже Choose a single. I think или выше is better. Хорошо иметь текущую версию ядра You're right)..But it's better to say: - Лучше использовать текущую версию ядра 2.4.х или 2.6.х Лучше всего компоновать Wine с инструментами GNU - компоновать Wine инструментами GNU Внимание : установка gas *НЕ* обеспечивает что gcc будет его использовать. - Внимание : установка gas *НЕ* гарантирует, что gcc будет его использовать. Перекомпилируйте gcc после установки gas или слинковать cc, as and ld to the gnu tools is said to be necessary. What's that? NetBSD: Будьте уверены что опции USER_LDT, SYSVSHM, SYSVSEM, и SYSVMSG включены в вашем ядре. - Убедитесь, что опции - SYSVSEM и SYSVMSG Мало проблем с совместимостью было получено используя файлы доступные через Samba. I think this should be paraphrased. Конфигурационный скрипт выведет список дополнительных библиотек которые не были найдены в вашей системе. - дополнительных библиотек, которые На 64-битных платформах вы должны быть уверены что у вас установлены 32-битные версии этих библиотек; смотрите http://wiki.winehq.org/WineOn64bit для деталей. - вы должны убедитсья, что; - за более подробной информацией обратитесь к http://wiki.winehq.org/WineOn64bit.; Затем введите make clean, и пропачтите релиз: - пропачьте Сначала не забудьте удалить любые конфликтные предыдущие установки Wine. Words missed. Посетите Службу подержки на http://www.winehq.org/ дополнительных сведений. Something missed too. wine notepad (using the search Path as specified in wine notepad.exethe config file to locate the file) Untranslated part here. Wine еще не завершен, поэтому некоторые программы могут вызывать ошибку. - Wine находится в состоянии разработки, поэтому некоторые программы могут вызывать ошибку. Во время ошибки вы будете направлены в отладчик для того чтобы вы могли исследовать и решить проблему. - в отладчик для того, чтобы вы могли I think all main problems are here. Maybe some further revise is needed after commit so feel free to improve it please.
Re: gdiplus/graphicspath test crashes
Adam Petaccia wrote: On Wed, 2008-06-25 at 10:41 +0200, Paul Vriens wrote: Hi, I've noticed that the mentioned tests crashes when run via Paul Millar's winetest executable. Compiling myself and running the test works fine. Any one else seeing this? I've noticed it crashes if Wine is compiled with -O0, but not when with -O2 (the default in most cases). This has been fixed by Nikoley's patch (http://source.winehq.org/git/wine.git?a=commit;h=991e785f507c13d6ebe648e650ff1ae3346ca2cd) -- Cheers, Paul.
Re: quartz Rewrite the avi splitter so that video and sound run in separate threads, with tests
Hi Detlef, 2008/6/26 Detlef Riekenberg [EMAIL PROTECTED]: On Mi, 2008-06-25 at 21:34 -0700, Maarten Lankhorst wrote: Because of the magnitude of the change I cannot split this patc hup, my apologies for this. Without any view at your code, the sentense above is wrong by design. A Test is always independant from the implementation, and what permit a seperate patch for test_threads() to split the tests? Oops, you're right, I missed that. :-) I'll split it up later, there are still some things I need to figure out with avi. Cheers, Maarten.