December 2009 - Wine-Devel - list.winehq.org

Re: ddraw: Fix an incorrect refcount test
by Stefan Dösinger Dec. 17, 2009

Dec. 17, 2009

Am 17.12.2009 um 20:20 schrieb Luke Benstead: > Luke. > <0001-ddraw-Fix-an-incorrect-refcount-test.txt> This patch is correct. The test intends to test DDraw7, there's a DDraw4 test below. So the variable name was wrong, not the macro.

1 0

Re: [shell32/tests] Initialize an APPBARDATA struct (Valgrind)
by Alexandre Julliard Dec. 17, 2009

Dec. 17, 2009

Paul Vriens <paul.vriens.wine(a)gmail.com> writes: > Fixes some Valgrind warnings. > > Changelog > Initialize an APPBARDATA struct (Valgrind) That's just hiding the problem, an app can pass the same structure. -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: [usrmarshal] Adjust new tests to pass on win9x platforms.
by Jacek Caban Dec. 17, 2009

Dec. 17, 2009

Hi Jeremy, On 12/17/09 12:13 AM, Jeremy White wrote: > --- > dlls/oleaut32/tests/usrmarshal.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/dlls/oleaut32/tests/usrmarshal.c b/dlls/oleaut32/tests/usrmarshal.c > index e9c53e9..1c770de 100644 > --- a/dlls/oleaut32/tests/usrmarshal.c > +++ b/dlls/oleaut32/tests/usrmarshal.c > @@ -306,7 +306,8 @@ static void test_marshal_LPSAFEARRAY(void) > buffer = HeapAlloc(GetProcessHeap(), 0, size); > init_user_marshal_cb(&umcb,&stub_msg,&rpc_msg, buffer, size, MSHCTX_DIFFERENTMACHINE); > next = LPSAFEARRAY_UserMarshal(&umcb.Flags, buffer,&lpsa); > - ok(next - buffer == expected, "Marshaled %u bytes, expected %u\n", (ULONG) (next - buffer), expected); > + ok(next - buffer == expected || next - buffer + sizeof(DWORD) == expected /*win9x*/, > + "Marshaled %u bytes, expected %u\n", (ULONG) (next - buffer), expected); > It looks like a good place to use broken(). Thanks, Jacek

3 2

Re: [PATCH 6/7] urlmon: Report redirected URL to object created by BindToObject.
by Paul Vriens Dec. 17, 2009

Dec. 17, 2009

On 12/15/2009 09:16 PM, Jacek Caban wrote: > --- > dlls/urlmon/binding.c | 6 +++++ > dlls/urlmon/tests/url.c | 58 ++++++++++++++++++++++++++++++---------------- > 2 files changed, 44 insertions(+), 20 deletions(-) > Hi Jacek, This one introduced a test failure: http://test.winehq.org/data/tests/urlmon:url.html I didn't find anything common for the boxes/OS, race? Could you have a look? -- Cheers, Paul.

4 4

Re: [PATCH] secur32: Fix memory leak in SECUR32_addPackages. (valgrind)
by James Hawkins Dec. 17, 2009

Dec. 17, 2009

On Wed, Dec 16, 2009 at 5:07 PM, Lei Zhang <thestig(a)google.com> wrote: > Hi, this is for bug 21045. > + SecurePackage *prev_package = NULL; LIST_FOR_EACH_ENTRY(package, &packageTable->table, SecurePackage, entry) { + if (prev_package) + HeapFree(GetProcessHeap(), 0, prev_package); + prev_package = package; HeapFree(GetProcessHeap(), 0, package->infoW.Name); HeapFree(GetProcessHeap(), 0, package->infoW.Comment); } + if (prev_package) + HeapFree(GetProcessHeap(), 0, prev_package); I believe the construct you're looking for is LIST_FOR_EACH_SAFE. grep through the code for examples on using that to safely free and remove elements from a list. -- James Hawkins

2 1

Re: [1/2] gdiplus: Explicitly copy the bits from dib sections to new bitmaps.
by Paul Vriens Dec. 17, 2009

Dec. 17, 2009

On 12/13/2009 12:59 AM, Vincent Povirk wrote: > Hi Vincent, It appears that this one introduced a test failure: http://test.winehq.org/data/tests/gdiplus:image.html One of the tests that fails now (line 533) was there for a long time it's just that you've added some stuff before that. The other one (line 1058) is because of the other patch "Add test for image palette functions." Could you have a look? -- Cheers, Paul.

2 5

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
by Juan Lang Dec. 16, 2009

Dec. 16, 2009

Hi Amine, this patch has no functional benefit. For example, - HRESULT hr; TRACE("\n"); - hr = SMTPTransport_ParseResponse(This, pBuffer, &response); - if (FAILED(hr)) + if (FAILED(SMTPTransport_ParseResponse(This, pBuffer, &response))) This has the dubious benefit of removing a local variable that was used before. I can't fathom why this is a good thing. --Juan

4 7

Re: [PATCH] inetcomm/smtptransport.c : Fix a typo ?
by Juan Lang Dec. 16, 2009

Dec. 16, 2009

Hi Amine, -static void SMTPTransport_CallbackMessageSendTo(IInternetTransport *iface, char *pBuffer, int cbBuffer); - > The actual function is a little bit under. Um, it's called a prototype, and it's needed to compile smtptransport.c. Have you tested this patch at all? --Juan

2 2

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
by Austin English Dec. 16, 2009

Dec. 16, 2009

On Wed, Dec 16, 2009 at 12:28 PM, Amine Khaldi <amine48rz(a)gmail.com> wrote: > WBR, > Amine Khaldi. Out of curiosity, what tool are you using to find these bugs? -- -Austin

3 3

Re: hlink/hlink_main.c : Remove an unneeded assignment.
by Jacek Caban Dec. 16, 2009

Dec. 16, 2009

Hi Amine, > r = register_clsid(&CLSID_StdHlink); > if (SUCCEEDED(r)) > - r = register_clsid(&CLSID_StdHlinkBrowseContext); > + register_clsid(&CLSID_StdHlinkBrowseContext); > > return S_OK; In this case the correct fix is to return r. Please be more careful then doing janitorial work. There are more places in your patches that seem questionable. You should make sure that we want to ignore an error instead of handling it (and usually we don't) before deleting such assignments. Otherwise it's just hiding bugs. Thanks, Jacek

2 1

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
by Austin English Dec. 16, 2009

Dec. 16, 2009

On Wed, Dec 16, 2009 at 12:28 PM, Amine Khaldi <amine48rz(a)gmail.com> wrote: > WBR, > Amine Khaldi. Out of curiosity, what tool are you using to find these bugs? -- -Austin

1 0

Re: [PATCH 07/10] urlmon: Added PARSE_DOMAIN to CoInternetParseUrl
by Paul Vriens Dec. 16, 2009

Dec. 16, 2009

On 12/15/2009 11:47 PM, Piotr Caban wrote: > --- > dlls/urlmon/internet.c | 32 ++++++++++++++++++++++++++++++++ > dlls/urlmon/tests/misc.c | 23 +++++++++++++++++------ > 2 files changed, 49 insertions(+), 6 deletions(-) > > Hi Piotr, This one has introduced some test failures: http://test.winehq.org/data/tests/urlmon:misc.html Looks more OS than IE specific? Could you have a look? -- Cheers, Paul.

1 0

chromium wine valgrind bot up...
by Dan Kegel Dec. 16, 2009

Dec. 16, 2009

More progress on my http://wiki.winehq.org/UnitTestSuites effort: Lei and I are running a buildbot that runs win32 chromium's unit tests under wine under valgrind. It just came up yesterday, and we're still working out some kinks, but you can see it at http://build.chromium.org/buildbot/waterfall.fyi/builders/Chromium%20Linux%… Running a real app's test suite under valgrind+wine can find both bugs in wine and bugs in the app; here are examples of both found today by that bot: http://bugs.winehq.org/show_bug.cgi?id=21041 (mismatched GlobalLock/HeapFree in wine?) http://code.google.com/p/chromium/issues/detail?id=30584 (app forgot to initialize a bitmap?) It'll be interesting to see what else this turns up over time.

1 0

Re: [try 2] [2/7] mapi32: Correct the Simple MAPI parameters
by Henri Verbeet Dec. 16, 2009

Dec. 16, 2009

2009/12/16 Owen Rudge <orudge(a)codeweavers.com>: > +214 stdcall MAPIFindNext(ptr ptr str str long long str) The last parameter is an output parameter.

1 0

Re: [2/5] winemapi: Add Simple MAPI functions
by Henri Verbeet Dec. 16, 2009

Dec. 16, 2009

2009/12/15 Owen Rudge <orudge(a)codeweavers.com>: > + 1 stdcall MAPIAddress(ptr ptr ptr long ptr long long ptr long ptr ptr) > + 6 stdcall MAPILogoff(long long long long) > +11 stdcall MAPISendMail(long long ptr long long) These don't look quite right to me. Somewhat related, LHANDLE is a ULONG in include/mapi.h, but a ULONG_PTR in my copy of the PSDK.

2 1

Re: msvcrtd: free mem after use (valgrind)
by Paul Vriens Dec. 16, 2009

Dec. 16, 2009

On 12/16/2009 03:25 PM, André Hentschel wrote: > --- > dlls/msvcrtd/tests/debug.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/dlls/msvcrtd/tests/debug.c b/dlls/msvcrtd/tests/debug.c > index 71f6e4c..dc89230 100644 > --- a/dlls/msvcrtd/tests/debug.c > +++ b/dlls/msvcrtd/tests/debug.c > @@ -60,6 +60,8 @@ static void test_new(void) > > mem = pMSVCRTD_operator_new_dbg(42, _NORMAL_BLOCK, __FILE__, __LINE__); > ok(mem != NULL, "memory not allocated\n"); > + if(mem != NULL) > + HeapFree(GetProcessHeap(), 0, mem); > } Hi André, There is no need to check before calling HeapFree. -- Cheers, Paul.

2 1

Re: msvcrtd: free mem after use (valgrind)
by Nikolay Sivov Dec. 16, 2009

Dec. 16, 2009

On 12/16/2009 17:25, André Hentschel wrote: > --- > dlls/msvcrtd/tests/debug.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/dlls/msvcrtd/tests/debug.c b/dlls/msvcrtd/tests/debug.c > index 71f6e4c..dc89230 100644 > --- a/dlls/msvcrtd/tests/debug.c > +++ b/dlls/msvcrtd/tests/debug.c > @@ -60,6 +60,8 @@ static void test_new(void) > > mem = pMSVCRTD_operator_new_dbg(42, _NORMAL_BLOCK, __FILE__, __LINE__); > ok(mem != NULL, "memory not allocated\n"); > + if(mem != NULL) > + HeapFree(GetProcessHeap(), 0, mem); > } > > /**********************************************************************/ > Pointer check is redundant in this case.

1 0

Re: [PATCH 2/3] mmdevapi: Add stubs for MMDevEnum, with tests
by Alexandre Julliard Dec. 16, 2009

Dec. 16, 2009

Maarten Lankhorst <m.b.lankhorst(a)gmail.com> writes: > +static ULONG WINAPI MMDevEnum_Release(IMMDeviceEnumerator *iface) > +{ > + MMDevEnumImpl *This = (MMDevEnumImpl*)iface; > + LONG ref = InterlockedDecrement(&This->ref); > + TRACE("Refcount now %i\n", ref); > + return ref; > +} Sounds like you want a static object here too... -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [3/5] WineD3D: Prepare for dynamic vertex buffers
by Henri Verbeet Dec. 16, 2009

Dec. 16, 2009

2009/12/15 Stefan Dösinger <stefan(a)codeweavers.com>: > + /* buffer_get_sysmem can remove the BO from dynamic buffers */ No, it can't? > @@ -540,8 +547,8 @@ static HRESULT WINAPI IWineD3DDeviceImpl_CreateVertexBuffer(IWineD3DDevice *ifac > TRACE("Not creating a vbo because GL_ARB_vertex_buffer is not supported\n"); > } else if(Pool == WINED3DPOOL_SYSTEMMEM) { > TRACE("Not creating a vbo because the vertex buffer is in system memory\n"); > - } else if(Usage & WINED3DUSAGE_DYNAMIC) { > - TRACE("Not creating a vbo because the buffer has dynamic usage\n"); > + } else if(!gl_support_dynamic_buffer(gl_info) && (Usage & WINED3DUSAGE_DYNAMIC)) { > + TRACE("Not creating a vbo because the buffer has dynamic usage but GL doesn't support dynamic buffers\n"); > } else if(!(Usage & WINED3DUSAGE_OPTIMIZE) && conv) { > TRACE("Not creating a vbo because the fvf needs conversion, but VB optimization is disabled\n"); > } else { It seems to me that this and similar code in CreateIndexBuffer() should be in buffer_init() instead.

2 1

Edit control messages through Comctl32 v6 module
by Nikolay Sivov Dec. 16, 2009

Dec. 16, 2009

Hi. Yesterday I looked at XP introduced messages for Edit controls (EM_SETCUEBANNER/EM_GETCUEBANNER). This definitely doesn't work without v6 module loaded. Does someone have any idea how this supposed to work? Any comments are welcome as usual. P.S. Yes, I know that comctl32 does subclass user32 control but only if theming is active (I mean Wine behavior), anyway this subclass procedure only cares about painting messages, control specific messages require control data manipulation, and this isn't a preferable of course.

4 18

Re: [PATCH 10/10] urlmon: Added tests for CoInternetGetSecurityUrl
by Paul Vriens Dec. 16, 2009

Dec. 16, 2009

On 12/15/2009 11:48 PM, Piotr Caban wrote: > --- > dlls/urlmon/tests/sec_mgr.c | 31 +++++++++++++++++++++++++++++++ > 1 files changed, 31 insertions(+), 0 deletions(-) > > Hi Piotr, Is there a way to avoid using lstrmcpW as that will always fail (or the !lstrcmpW case will always succeed) on Win9x/WinMe? It looks like the rest of this file use memcmp to compare strings. -- Cheers, Paul.

2 2

Re: [10/10] rpcrt4: Add tests for RPC calls with authentication.
by Paul Vriens Dec. 16, 2009

Dec. 16, 2009

Hi Rob, I took the liberty of running these tests on winetestbot. It shows two errors for W2K and above: https://winetestbot.geldorp.nl/JobDetails.pl?Key=115 -- Cheers, Paul.

3 7

strangeness under Dec 14th Wine Git
by chris ahrendt Dec. 15, 2009

Dec. 15, 2009

Just a little nit I noticed with yesterdays Git tree... When you give a window other than the wine window the focus... the wine window does not go into the background... so IF I have 2 or 3 windows open with a wine app running and click on say thunderbird... or firefox to look something up I have to move the wine window to the bottom of the screen. It looses focus (turns grey) but does not go to the background... Chris

1 0

[rfc] Provide separate operator vector new and delete in msvcrt; let valgrind check them
by Dan Kegel Dec. 15, 2009

Dec. 15, 2009

First draft for discussion. Not, um, well tested yet. ANSI C++ mandates separate vector versions of new and delete that do the same thing as the non-vector versions, but which can be replaced by user apps (see http://www.csci.csusb.edu/dick/c++std/cd2/lib-support.html#lib.new.delete ) So far, so good; wine does that in msvcrt. However, once in a while, a programmer will call delete when he should have called delete[], and a bunch of destructors won't get called. To detect this programming error, let's split the vector new and delete functions off from their scalar twins, and add values for the heap function flags so we can inform valgrind what kind of blocks are being wrangled. This means picking two flag bits that Microsoft doesn't use; there seem to be enough to go around. Requires the corresponding patch to valgrind ttps://bugs.kde.org/show_bug.cgi?id=218741 Thanks to Steve Vandebogart for his help on this; he wrote a bunch of it.

2 1

Tests for GetLongPathName
by Paul Vriens Dec. 15, 2009

Dec. 15, 2009

Hi Alexandre, Several file related functions have a maximum path size of 32767 when using a "\\?\" prefix. Testing these large paths only works on Windows due to Linux having a maximum path limit of 4096. Does it still make sense to add these tests or should I focus on <= 4096? I guess if we/Wine have a limit due to the OS we are running on we should probably act the same as that higher limit on Windows? -- Cheers, Paul.

2 1