- Wine-Devel - list.winehq.org

RE: -DSTRICT warning fixes
by Patrik Stridvall July 28, 2001

July 28, 2001

> Patrik Stridvall <ps(a)leissner.se> writes: > > > However for the debugging output I disagree. I orginally thought > > as Francois (and you it seems) that handles are pointers and should > > be treated as such. But then I realized that when we > implement Win32 on > > 64-bit platforms we can't have handles as pointers even > internally if > > we wan't to support x86 emulation and even exposing a 64-bit > > handle to Winelib application can only cause trouble since they > > might store it is a 32-bit memory location. Sure we will probably be > > able to handle 64-bit pointers truncated to 32-bit by storing all > > objects in the lower 32-bit part of memory, but fear it might > > cause trouble all the same. > > Handles are pointers in the Windows API, and they should be pointers > inside of Wine. On a 64-bit platforms pointers will be 64-bit, and so > will handles. Does the Windows API really define exactly what a handle is? It is just a identifier for an object that can be passed to various APIs but should not be used directly. > If you want to run 32-bit code on such a platform you > need a conversion for pointers anyway, > so you might as well use the > same conversion for handles. > Not to mention that some handles are > actually pointers (like HMODULE). That is an internal Wine/Winelib problem and shouldn't need to concern porters of Winelib applications. > So no, there is absolutely no reason > to force handles to be 32 bits. Handles are some sort of values as for as whether they are integers or pointers any Winelib application that cares are IMHO broken. I guess you agree on that point. However your suggestion that any Winelib application that cares whether they are 32-bit or 64-bit are broken is a far too strict IMHO and I see no reason to require porters to fix such things. Note that it might no be so easy to fix it either. Perhaps the application uses some external lib that provides storage of some user defined 32-bit data and use it to store the handles. Now it will suddenly not work anymore, and fixing it is not trival. Hint you are on a 64-bit platform that requires 64-bit pointers and you have 32-bit storage. The obvious solution an index of an array might not always work so well because of memory management requirements and beside the maintainer of the project might not accept a special patch for the 64-bit version. Anyway, never mind. See below instead. Sure pointer face in some meaning the same problems, but that sort of problems is more obvious and there is no reason to add more to the burden. > > The real reason that we should support compiling Wine with -DSTRICT > > is not because handles becomes pointers but because callbacks gets > > the correct prototype. > > No, it's because handles become pointers; you cannot do the -DSTRICT > typechecking with integers anyway. True, you can't do typechecking with integers but since Wine is primarily compiled 32-bit platform that will use 32-bit pointers, we will get the benefit of typechecking at the platform, which happends the only one Wine currently works on. Sure future developers on 64-bit will not find the errors then they compile, but everybody else compiling for a 32-bit platform will, so I don't see the problem. > Callbacks are a very minor issue; > the real point is to fix all incorrect handle manipulations, like > implicit 16-bit handle conversions or arithmetic on handle values. The ones of the above that can be found automatically will be fixed by the 32-bit developers. So the actual problem is what? Anyway, this discusion is getting a little carried away (mostly my fault). While I do not currenly agree that we really should use 64-bit handles on 64-bit platforms, your objections have made me think some more and made me come to the following conclusions. In order to not "close the door to the future" whether this is Win32 on 64-bit or Win64, the Wine code shouldn't internally not care (give errors or warnings or be semantically wrong) whether the header files: 1. Defines handles as integers or pointers 2. Defines handles as 32-bit or 64-bit (regardless of whether we have 32-bit or 64-bit pointers) This is not so a for the normal (non-debugging) code. OK, we might have to force memory allocation of objects for the lower 32-bit of memory if we have 64-bit pointers but that is probably not so large problem. After all the Winelib application itself can allocate memory on the full 64-bit memory range and Winelib itself are not likely to need more that 4Gb memory for objects. However, I see no really good solution for the debug output. Neither mine nor your previously suggested solution have the above properties. However the solution below have, I think TRACE("(%p)", (LPVOID) handle); eventhough it might display unnessary information on 64-bit platforms, but perhaps we should worry about that in the future, if it indeed is a problem at all. After all it is only a few more bytes in the debug output. So therefor I suggest that we should do the following TRACE("(%p)", (LPVOID) handle); It will fix the problem that Francois speaks of in bug report #90. < * Ignore gcc's warnings saying '%x format, pointer argument'. Fixing the format < at this stage would only introduce tons of warnings for the developpers not < participating in the switch (i.e. compiling with STRICT off). It will allow us to fix the debug output one by one (or at least seperately per type) since it will allow us compile without related warnings even without -DSTRICT as well as to prepare us for a possible future where handles might be 32-bit integers on 64-bit.

1 0

RE: -DSTRICT warning fixes
by Patrik Stridvall July 28, 2001

July 28, 2001

> On Sat, 28 Jul 2001, Patrik Stridvall wrote: > [...] > > The real reason that we should support compiling Wine with -DSTRICT > > is not because handles becomes pointers but because callbacks gets > > the correct prototype. > > > > I case not you haven't realized the obvious, it doesn't matter if > > the handles are 32-bit integer or 32-bit pointers as long as > > all handles are the same. The code either > > No, I think you have missed something: when compiling with -DSTRICT > on all handles are not the same. Note I said same but meant compatible. Wrong choice of words. > For instance you cannot do > hDesk=hAccel. That's because with -DSTRICT on you have: > > typedef struct HDESK__ { int unused; } *HDESK; > typedef struct HACCEL__ { int unused; } *HACCEL; > > Thus HDESK and HACCEL are incompatible pointer types. This > brings us > more type safety which is the whole point of -DSTRICT. Oh sure, but the point is that if code, that are semanticly correct, cares whether you are strict or relax the type, the code it wrong. Perhaps I was I little unclear on that point. Being strict obviously makes it easier to find semantic errors, but that was not the point at all. > About 64bits Windows I don't know yet how we are going to do that. > But you have to check exactly what Windows 64bit does, not just > extrapolate from the way Unix 64bit does things. AFAIK the > approach used > by windows is quite different from that which Unices took. Yes, unfortunately, but I'm primarily worried about Win32 on 64-bit platforms.

1 0

RE: -DSTRICT warning fixes
by Patrik Stridvall July 28, 2001

July 28, 2001

> > If we do that can just do > > s/DECLARE_OLD_HANDLE/DECLARE_HANDLE/ > > on the handle types on by one and fix the warnings. > > That's precisely the plan, and it is just the opposite of what you are > doing with your patch (you are turning DECLARE_HANDLE back into > DECLARE_OLD_HANDLE). The reason I did that was that they can't be fixed as long as HANDLE/HGLOBAL is the wrong types. When they become the right types the warnings will automatically disappear and we can do s/DECLARE_OLD_HANDLE/DECLARE_HANDLE/. But there is no reason to have a lot of warnings that can't be fixed otherwise until then. > Yes you get a number of warnings if you turn > -DSTRICT on, most of them should disappear once the conversion is > complete; fixing them before that is a waste of time. Since we can't fix before that why show them at all? People that want to fix for example output messages can do, s/DECLARE_OLD_HANDLE/DECLARE_HANDLE/ in their tree, fix them and submit the patches (without the s/DECLARE_OLD_HANDLE/DECLARE_HANDLE/). > This is the > reason why -DSTRICT is not the default yet. Read bug report #90, > Francois explains everything there. OK. I have read bug report #90 now. > And don't forget the number one rule of the -DSTRICT conversion is: > *no* *typecasts*. If you have to add typecasts something is wrong. No typecasts where the handle is used in conjuction with other handles, certainly not, no argument there. The code some expect all handles to be the of same type. However for the debugging output I disagree. I orginally thought as Francois (and you it seems) that handles are pointers and should be treated as such. But then I realized that when we implement Win32 on 64-bit platforms we can't have handles as pointers even internally if we wan't to support x86 emulation and even exposing a 64-bit handle to Winelib application can only cause trouble since they might store it is a 32-bit memory location. Sure we will probably be able to handle 64-bit pointers truncated to 32-bit by storing all objects in the lower 32-bit part of memory, but fear it might cause trouble all the same. Beside doing TRACE("(%p)", handle) on 64-bit platforms will needlessly display the always the same presumably zero upper 32-bit part of the handle. That why I decided to should use TRACE("(0x%08lx)", (DWORD) handle) instead to avoid such issues. In short, the code should IMHO support both 32-bit pointers as well and 32-bit integers as handles presuming of course that all handles use the same. The real reason that we should support compiling Wine with -DSTRICT is not because handles becomes pointers but because callbacks gets the correct prototype. I case not you haven't realized the obvious, it doesn't matter if the handles are 32-bit integer or 32-bit pointers as long as all handles are the same. The code either 1. Just pass them around as the same type of handles of other types of handles (HANDLE/HGLOBAL). 2. Tries to access the underlying object, in such case a void pointer will not do use any good, we must use a cast either after calls to functions like GlobalLock or alone.

2 1

Re: -DSTRICT warning fixes
by Alexandre Julliard July 28, 2001

July 28, 2001

Patrik Stridvall <ps(a)leissner.se> writes: > However for the debugging output I disagree. I orginally thought > as Francois (and you it seems) that handles are pointers and should > be treated as such. But then I realized that when we implement Win32 on > 64-bit platforms we can't have handles as pointers even internally if > we wan't to support x86 emulation and even exposing a 64-bit > handle to Winelib application can only cause trouble since they > might store it is a 32-bit memory location. Sure we will probably be > able to handle 64-bit pointers truncated to 32-bit by storing all > objects in the lower 32-bit part of memory, but fear it might > cause trouble all the same. Handles are pointers in the Windows API, and they should be pointers inside of Wine. On a 64-bit platforms pointers will be 64-bit, and so will handles. If you want to run 32-bit code on such a platform you need a conversion for pointers anyway, so you might as well use the same conversion for handles. Not to mention that some handles are actually pointers (like HMODULE). So no, there is absolutely no reason to force handles to be 32 bits. > Beside doing TRACE("(%p)", handle) on 64-bit platforms will needlessly > display the always the same presumably zero upper 32-bit part > of the handle. That why I decided to should use > TRACE("(0x%08lx)", (DWORD) handle) instead to avoid such issues. Sorry but this is wrong. Handles are pointers and must be printed as pointers. Assuming that handles are always exactly 32-bit is very shortsighted. > The real reason that we should support compiling Wine with -DSTRICT > is not because handles becomes pointers but because callbacks gets > the correct prototype. No, it's because handles become pointers; you cannot do the -DSTRICT typechecking with integers anyway. Callbacks are a very minor issue; the real point is to fix all incorrect handle manipulations, like implicit 16-bit handle conversions or arithmetic on handle values. -- Alexandre Julliard julliard(a)winehq.com

1 0

Re: -DSTRICT warning fixes
by Alexandre Julliard July 28, 2001

July 28, 2001

Patrik Stridvall <ps(a)leissner.se> writes: > Concerning some parts of my warnings patch that was > partly rejected I disagree with the rejection. > The patch in question is in warnings_rejected.diff. > Note the field "u" is a union. But perhaps you know > and rejected it for other reasons? I know it is a union; I thought it was one of these cases that would get fixed by -DSTRICT, but now I see that it is a slightly different problem. I'll apply it. > If you accept all the strict_*.diff patches, perhaps we > should consider adding -DSTRICT as a permanent flag? > It removes the HOOKPROC/TIMERPROC warnings that is not > possible to fix otherwise. > > If we do that can just do > s/DECLARE_OLD_HANDLE/DECLARE_HANDLE/ > on the handle types on by one and fix the warnings. That's precisely the plan, and it is just the opposite of what you are doing with your patch (you are turning DECLARE_HANDLE back into DECLARE_OLD_HANDLE). Yes you get a number of warnings if you turn -DSTRICT on, most of them should disappear once the conversion is complete; fixing them before that is a waste of time. This is the reason why -DSTRICT is not the default yet. Read bug report #90, Francois explains everything there. And don't forget the number one rule of the -DSTRICT conversion is: *no* *typecasts*. If you have to add typecasts something is wrong. -- Alexandre Julliard julliard(a)winehq.com

1 0

(no subject)
by Ladislav Sladecek July 28, 2001

July 28, 2001

Alexandre Julliard: > I think GetClipBox is correct, the problem is > that the WM_ERASEBKGND > handler should not be using it, but > GetClientRect. You can try this: Hello Alexandre, thank you for your reccomendation. Unfortunately the DefWindowProc improvement does not help, because the default handler newer gets called in my app. The application (Progress _prowin.exe version 7) uses custom dialog boxes (forms) and also custom controls. The controls handle WM_ERASEBKGND themeselves. I have no access to the sources. What I know about GetClipRect, I know from the trace. I agree that your method (GetClientRect in the handler) is much better, but I cannot use it. I also agree that Windows should return the parent area in GetClipBox as Wine does. Unfortunalelly, the Windows do not do that while the _prowin.exe depends on it. I enclose a simple C example which I hacked to demonstrate this. The example displays two red children in a white parent. The value returned by GetClipBox is wsprintf-ed. If you run the binary in the Windows, you will see two properly sized red rectangles on a white background. If you run the same binary in Wine, you will see only one large red child but no white parent background. If either the CS_PARENTDC or WS_CLIPSIBLINGS gets removed from my example, both Windows and Wine give identical results. Regards Ladislav Sladecek

1 0

Re: -DSTRICT warning fixes
by Francois Gouget July 28, 2001

July 28, 2001

- FIXME("(%x),stub!\n",hrasconn); + FIXME("(0x%08lx): stub\n", (DWORD) hrasconn); [...] I think I would rather use %p to print handles. After all they are pointers, not integers. The annoying aspect of it is that %08p is not very well supported IIRC. Maybe gcc has been fixed though. I forgot to mention it before. But I assume you saw bug #90. -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ Advice is what we ask for when we already know the answer but wish we didn't -- Eric Jong

1 0

wine on clusters ??
by Francisco Perez July 27, 2001

July 27, 2001

Hi everybody: Has anybody tried to run wine on a High Availabity cluster ?. Especifically we are interested on a Mosix environment where applications don't need be changed. The windows application to be clustered is a Remote IP network back-up system so no grafic intensive or plug modern like ActiveX and so. Any comments are highly apreciated thanks

1 0

Re: PATCH: wrong clipRect when CS_PARENTDC and WS_CLIPSIBLINGS styles are combined
by Alexandre Julliard July 27, 2001

July 27, 2001

lsla(a)post.cz writes: > Where the Wine's GetClipRect returns the area of the parent, the > Windows version returns only the area of the child. Therefore in > Wine the WM_ERASEBKGND message erases the whole parent including the > other radio buttons. I think GetClipBox is correct, the problem is that the WM_ERASEBKGND handler should not be using it, but GetClientRect. You can try this: Index: windows/defwnd.c =================================================================== RCS file: /opt/cvs-commit/wine/windows/defwnd.c,v retrieving revision 1.57 diff -u -r1.57 defwnd.c --- windows/defwnd.c 2001/06/20 23:16:34 1.57 +++ windows/defwnd.c 2001/07/27 23:47:26 @@ -457,20 +457,15 @@ case WM_ICONERASEBKGND: { RECT rect; + HDC hdc = (HDC)wParam; HBRUSH hbr = GetClassLongW( wndPtr->hwndSelf, GCL_HBRBACKGROUND ); if (!hbr) return 0; - /* Since WM_ERASEBKGND may receive either a window dc or a */ - /* client dc, the area to be erased has to be retrieved from */ - /* the device context. */ - GetClipBox( (HDC)wParam, &rect ); - - /* Always call the Win32 variant of FillRect even on Win16, - * since despite the fact that Win16, as well as Win32, - * supports special background brushes for a window class, - * the Win16 variant of FillRect does not. - */ - FillRect( (HDC) wParam, &rect, hbr ); + /* GetClientRect used to be GetClipBox, but it is not what + * Windows does, and it breaks badly with CS_PARENTDC */ + GetClientRect( wndPtr->hwndSelf, &rect ); + DPtoLP( hdc, (LPPOINT)&rect, 2 ); + FillRect( hdc, &rect, hbr ); return 1; } -- Alexandre Julliard julliard(a)winehq.com

1 0

RE: winapi update
by Patrik Stridvall July 27, 2001

July 27, 2001

> make_filter: : can't parse output: 'Make.rules.in is newer > than Make.rules, please rerun ./configure!' Fixed in my tree. > Would it be possible to have the messages pass through as-is when > they are not recognized (with the current directory for context)? [snip] > make_filter: gcc: can't parse output: 'd3ddevice/mesa.c:817: > warning: unused variable `xvis'' I have fixed that for gcc messages (unless --pedantic is specified), however I'm not sure it is good always doing that. I will think about it. > It looks like there's an incompatibility with colorgcc. [snip] > So it must be the controls characters added by colorgcc > that confuse > make_filter. No it wasn't, colorgcc checks whether STDOUT is a TTY or not.

1 0