May 2009 - Wine-devel - list.winehq.org

malloc() use in rpcrt4/tests/server.c
by Francois Gouget 20 May '09

20 May '09

Is there a specific reason for using malloc() in these tests? Testing that rpcrt4 does not assume HeapAlloc pointers? (but why would it even care?) Here are the relevant lines: in midl_user_allocate(): dlls/rpcrt4/tests/server.c:56: use HeapXxx(): return malloc(n); in array_tests(): dlls/rpcrt4/tests/server.c:1212: use HeapXxx(): dc = malloc(FIELD_OFFSET(doub_carr_t, a[2])); dlls/rpcrt4/tests/server.c:1214: use HeapXxx(): dc->a[0] = malloc(FIELD_OFFSET(doub_carr_1_t, a[3])); dlls/rpcrt4/tests/server.c:1219: use HeapXxx(): dc->a[1] = malloc(FIELD_OFFSET(doub_carr_1_t, a[2])); -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Cahn's Axiom: When all else fails, read the instructions.

1 0

Re: [PATCH] winhttp: avoid NULL ptr dereference (Coverity 927)
by Paul Vriens 20 May '09

20 May '09

Marcus Meissner wrote: > Hi, > > Not sure if we need to set these values in all error paths, > but definitely not in the NULL lpwhr. (lpwhdr?) > > Ciao, Marcus > --- > dlls/wininet/http.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c > index 212ff9f..53c3743 100644 > --- a/dlls/wininet/http.c > +++ b/dlls/wininet/http.c > @@ -3831,8 +3831,9 @@ static BOOL HTTP_OpenConnection(LPWININETHTTPREQW lpwhr) > > TRACE("-->\n"); > > + if (!lpwhr) goto lend2; > > - if (NULL == lpwhr || lpwhr->hdr.htype != WH_HHTTPREQ) > + if (lpwhr->hdr.htype != WH_HHTTPREQ) > { > INTERNET_SetLastError(ERROR_INVALID_PARAMETER); > goto lend; > @@ -3894,6 +3895,7 @@ lend: > lpwhr->read_pos = lpwhr->read_size = 0; > lpwhr->read_chunked = FALSE; > > +lend2: > TRACE("%d <--\n", bSuccess); > return bSuccess; > } Hi Marcus, After AJ's remark you marked this Coverity one as "lpwhr can not be NULL". Should we thus delete the "NULL == lpwhr" altogether? -- Cheers, Paul.

2 1

Bugzilla upgrade - opening attachments in browser
by Austin English 20 May '09

20 May '09

It seems the bugzilla upgrade stopped allowing opening attachments in browser. Not sure if that's by choice or just an unchanged setting, but having to open .txt/.jpeg/.png files in outside applications is a bit annoying...Any possibility of fixing that? -- -Austin

2 1

user32: improvements
by Daniel Santos 19 May '09

19 May '09

In the process of working on this bug (http://bugs.winehq.org/show_bug.cgi?id=18371) I have discovered that user32's SetCursor function does not ensure that the the handle passed is valid or that it is even a handle to a cursor object (and it seems that the lingo for user32 objects is "resources"). The root cause of this problem is that the app attempts to use a cursor after it already destroyed it. At first, this just causes the cursor to be blank. Then, it would appear that the handle is re-used for a different type of resource, which leads to the gibberish image & subsequent hang. When this app runs on windows, I presume that the SetCursor calls just fail and the app resumes harmlessly ignoring the failure (or correcting it, either way). In general, this flaw will affect applications which use the user32 api irresponsibly, and I'm certain that a great number of windows apps out there, to some extent, do. My 1st step is to write some tests that illustrate/prove this problem (and that it works on windows) and I'm building the tests now so I can start on that. However, I've been reading up and considering how to implement a solution. I presume that this problem affects every user32 handle & function related to them, but I have only thoroughly examined mousecursor.c, so please forgive any incorrect assumptions I still have at this point (and let me know if I'm way off!). There was a quaint MSDN article on how the windows GDI32 dll manages GDI objects that even gave the internal structure, etc. (http://msdn.microsoft.com/en-us/magazine/cc188782.aspx) Although this only covers gdi32, I'm getting the idea that what needs to happen in user32 isn't too much different. Essentially, it would seem that user32 needs to associate a handle with a type and it would *appear* at this point that would be sufficient. However, I need to gain a better understanding of the nuisances of user32 objects.. From reading this ms kb article (http://support.microsoft.com/kb/184802) it seems that the windows user32 maintains 3 different types of heaps, a global one, one for each desktop and a different (differently sized?) one for non-interactive desktops. Windows' "global heap" is where user32 stores it's handle table, presumably which, simply associates a kernel32 handle with appropriate information for user32 objects, similar to the way GDI does (as illustrated in the above referenced MSDN article). Thus, if you create a thread, it can never have the same handle value as a cursor, icon, etc. So I'm going to write my tests on my windows machine, starting with only the mousecursor.c stuff because there appears to be quite a lot of ground to cover. If anybody has any feedback, corrections to anything I've said, etc. please do reply. Daniel

1 0

Re: Detecting an invalid handle in setupapi
by Daniel Santos 19 May '09

19 May '09

oops, I meant to reply-all on this. --- On Mon, 5/18/09, Daniel Santos <javatroubadour(a)yahoo.com> wrote: From: Daniel Santos <javatroubadour(a)yahoo.com> Subject: Re: Detecting an invalid handle in setupapi To: "Paul Vriens" <paul.vriens.wine(a)gmail.com> Date: Monday, May 18, 2009, 5:43 PM I'm so very glad you brought this up Paul. I'm new to wine development and I've been working on Lord of the Rings Online issues and I have discovered and validated that the root cause of two of my problems are related (http://bugs.winehq.org/show_bug.cgi?id=18371) There are numerous places in Wine where the validity of handles are not checked. If the handle turns out to be invalid, then a NULL pointer is simply passed instead (which sometimes has an alternate meaning). Worse yet, in the code I was working with (dlls/kernel32/global16.c) the type of object the handle refers to is not stored! I see this as a critical flaw in Wine and I hope I can come up with a viable solution. I'm planning on setting up some test programs to examine the bit-data of some windows objects (starting with user32 -- where my issue lies) as any solution will have to be bit-compatibile with windows since apps like paint shop pro de-reference handles and examine the data of the underlying objects. In truth, if the LOTRO app didn't destroy objects it was still using and then try to use them, the problem wouldn't exist -- this part of wine will work perfectly if the app behaved responsibly. But development timelines and pressures frequently lead to code that works, but is incorrect and is a part of reality. Daniel --- On Sat, 5/16/09, Paul Vriens <paul.vriens.wine(a)gmail.com> wrote: From: Paul Vriens <paul.vriens.wine(a)gmail.com> Subject: Detecting an invalid handle in setupapi To: "wine-devel(a)winehq.org" <wine-devel(a)winehq.org> Date: Saturday, May 16, 2009, 12:18 PM Hi, I'm trying to install Microsoft's MMC (W2K3) and it crashes every now and then with different symptoms. I've tracked it down to what I think is a fault in the application that triggers a bug in Wine. All inf files that are read are closed with SetupCloseInfFile but the app is still trying to use the handle in a call to SetupFindFirstLineA. On Windows (tested on W2K3) this will lead to a failure of SetupFindFirstLineA with a last error of ERROR_INVALID_HANDLE. On Wine we don't check the handle and thus the outcome cannot be predicted (mostly crash). What ways do we have to detect invalid handles or better yet how can this be solved? -- Cheers, Paul.

1 0

Re: winemaker: add include-directory parsing
by Paul Vriens 19 May '09

19 May '09

André Hentschel wrote: > --- > tools/winemaker | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > > > ------------------------------------------------------------------------ > > Hi André, Not a comment on the patch itself but this is the third time you tried with the same patch. We usually add a 'resend' to the subject of a patch that is, uuh resend. If it's another revision we add 'try x'. In your case you should probably ask (on wine-devel) for comments or contact AJ directly on irc. -- Cheers, Paul.

1 0

re: [Article] WINE and the importance of application compatibility
by Dan Kegel 19 May '09

19 May '09

James McKensie wrote: > WE need to get to the point where most of the common business type > applications will run in Wine without a bunch of twiddling and fixes. Well, yes. That was one of my goals during my big Wine push ( http://www.winehq.org/pipermail/wine-devel/2008-February/062550.html ), and I think we made a lot of progress. Still lots to do, as you know. Sure would be nice if we could convince the DoD they needed a second source for Windows :-) - Dan

9 10

Gecko installation broken?
by Stefan Dösinger 19 May '09

19 May '09

Hi, Is my setup broken, or does the gecko installation fail? In a new wine prefix, when I open builtin iexplore, I get the installation dialog. The download runs successfully, but after installation iexplore still says that HTML rendering is currently disabled. When I run iexplore again, I again see the Wine Gecko installer dialog box. My hacky nightly game tests ran into this problem. The last successfull Team Fotress 2 test run is from May 14, 2009. After that my Gecko setup script failed and did not even try to start up Steam. Cheers, Stefan

4 6

Re: ntdll/tests: Mark return value of RtlUnicodeStringToInteger("", 16) as broken for nt4.
by Paul Vriens 19 May '09

19 May '09

Huw Davies wrote: > --- > dlls/ntdll/tests/rtlstr.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > > ------------------------------------------------------------------------ > > Hi, I don't think this is correct. This test has always succeeded until March 9th. There haven't been any changes to the test in that period, but I remember an upgrade of winehq in that weekend. In other words, there is more to it. Testing a simple cross compiled version of this test doesn't yield an error on NT4. -- Cheers, Paul.

3 2

Re: gdiplus: implement GdipTransformPoints
by Nikolay Sivov 19 May '09

19 May '09

Vincent Povirk wrote: >From 1de9b2291bf9a5617ca1e3b0cba5f1260889c259 Mon Sep 17 00:00:00 2001 From: Vincent Povirk <vincent(a)codeweavers.com> Date: Mon, 23 Mar 2009 16:34:12 -0500 Subject: [PATCH] gdiplus: implement GdipTransformPoints It's a strange way to check enumeration with integer comparison, isn't it? And not clear in this particular case. > + if (stat == Ok) > + { > + unitscale = convert_unit(graphics->hdc, graphics->unit); > + > + if(graphics->unit != UnitDisplay) > + unitscale *= graphics->scale; > + > + if (src_space <= CoordinateSpaceWorld && dst_space >= CoordinateSpacePage) > + GdipMultiplyMatrix(matrix, graphics->worldtrans, MatrixOrderAppend); > + if (src_space <= CoordinateSpacePage && dst_space >= CoordinateSpaceDevice) > + GdipScaleMatrix(matrix, unitscale, unitscale, MatrixOrderAppend); > + > + if (dst_space <= CoordinateSpacePage && src_space >= CoordinateSpaceDevice) > + GdipScaleMatrix(matrix, 1.0/unitscale, 1.0/unitscale, MatrixOrderAppend); > + if (dst_space <= CoordinateSpaceWorld && src_space >= CoordinateSpacePage) > + { > + GpMatrix *inverted_transform; > + stat = GdipCloneMatrix(graphics->worldtrans, &inverted_transform); > + if (stat == Ok) > + { > + GdipInvertMatrix(inverted_transform); > + GdipMultiplyMatrix(matrix, inverted_transform, MatrixOrderAppend); > + GdipDeleteMatrix(inverted_transform); > + } > + Maybe it's better to use 'switch' for source coords with falling and ifs inside cases for destination. <= CoordinateSpaceWorld is just CoordinateSpaceWorld >= CoordinateSpaceDevice - is CoordinateSpaceDevice.

3 3