- Wine-Devel - list.winehq.org

RE: wine/port.h #include fixes
by Patrik Stridvall Oct. 15, 2001

Oct. 15, 2001

> François Gouget wrote: > > > > This is a followup on the previous config.h patch, this > time tackling > > the wine/port.h case. > > Hmmm, I see it has been applied to CVS already. But I'm > not too happy > with it anymore :-/ > > I found a problem with 'dlls/winsock/async.c' and > 'dlls/winsock/socket.c' and the DELETE macro. > * DELETE is defined in winnt.h > * DELETE is also defined by a socket-related Solaris header > (arpa/nameser_compat.h, included indirectly) > * so winnt.h undefs DELETE and then defines our value Yes. That is a problem. ERR IIRC is another problem on Solaris. That is why #include "debugtools.h" is always done after all other includes. > * 'wine/port.h' includes 'winnt.h'. So if we include it > first and then > include the Solaris header we get a redefinition error. No it doesn't. At least not any longer. >From "cvs log include/wine.h": < revision 1.15 < date: 2001/07/26 21:43:29; author: julliard; state: Exp; lines: +1 -1 < Patrik Stridvall <ps(a)leissner.se> < Removed inclusion of winnt.h because the porting layer shouldn't < include Windows specific stuff. Note that as the comment says we shouldn't include Windows specific stuff in the porting layer. > One possible solution would be to handle DELETE differently, e.g. > prefix it in Wine, and not prefix it for Winelib, or something like > that. But wine/port.h is not the only header that needs to include > 'config.h'. There is also: > > cdrom.h console.h > ts_shape.h ts_xf86dga.h ts_xf86dga2.h ts_xf86vmode.h ts_xlib.h > ts_xpm.h ts_xresource.h ts_xshm.h ts_xutil.h ts_xvideo.h > wine_gl.h > x11drv.h > > I think it would be wrong to move all these #includes so that they > are first. Plus for the ts_*.h headers I am not sure it would be very > good. Also #including config.h in these is a bit pointless > since we know > that the file including them will need to include config.h too. Sounds reasonable. > So I propose the following instead: > * config.h has no multiple include protection. Add one > (__WINE_CONFIG_H > macro). > * modify the above headers so that they don't include config.h but > contain the following instead: > #ifndef __WINE_CONFIG_H > # error You must include config.h to use this header > #endif > This will make sure that no-one forgets to include config.h when > necessary. > * modify all files that include the above headers so that > they include > config.h as the first header. This actually requires relatively little > work. That is probably a good idea. > * do the same for port.h > * debugtools.h also uses config.h because it needs NO_DEBUG_MSGS and > NO_TRACE_MSGS. Modify configure.in so that these are put on > the command > line instead. They are not the of the same nature as the other macros > anyway: they are completely system independent. Also this will make it > possible to use the Wine debug macros in Winelib applications. Then > debugtools.h will no longer need to #include config.h. This cuts the > number of files causing trouble in about half. That is probably a good idea too. > * there are 3 other files that #include config.h for no good reason > that I can see: gdi.h, heap.h and thread.h. Remove the #includes from > there. Anyway, winapi_check ensures that all the .c files that need > config.h already include it directly. Yes. However that algoritm (or rather heuristics) that decide this is not perfect but it catch most of the cases and the compiler catches a few more and the linker most (all?) of the rest. > * maybe modify winapi_check to make sure that no header file includes > 'config.h' (this would be the frosting on the cake) This is easy. I will do that if Alexandre accept a patch what requires this. > Doing this (except for port.h) I have a tree that builds on Solaris > with no _FILE_OFFSET_BITS warning. So if the above sounds good, I have > patches that are almost ready. Ah. Good.

1 0

Re: wine/port.h #include fixes
by François Gouget Oct. 15, 2001

Oct. 15, 2001

François Gouget wrote: > > This is a followup on the previous config.h patch, this time tackling > the wine/port.h case. Hmmm, I see it has been applied to CVS already. But I'm not too happy with it anymore :-/ I found a problem with 'dlls/winsock/async.c' and 'dlls/winsock/socket.c' and the DELETE macro. * DELETE is defined in winnt.h * DELETE is also defined by a socket-related Solaris header (arpa/nameser_compat.h, included indirectly) * so winnt.h undefs DELETE and then defines our value * 'wine/port.h' includes 'winnt.h'. So if we include it first and then include the Solaris header we get a redefinition error. One possible solution would be to handle DELETE differently, e.g. prefix it in Wine, and not prefix it for Winelib, or something like that. But wine/port.h is not the only header that needs to include 'config.h'. There is also: cdrom.h console.h ts_shape.h ts_xf86dga.h ts_xf86dga2.h ts_xf86vmode.h ts_xlib.h ts_xpm.h ts_xresource.h ts_xshm.h ts_xutil.h ts_xvideo.h wine_gl.h x11drv.h I think it would be wrong to move all these #includes so that they are first. Plus for the ts_*.h headers I am not sure it would be very good. Also #including config.h in these is a bit pointless since we know that the file including them will need to include config.h too. So I propose the following instead: * config.h has no multiple include protection. Add one (__WINE_CONFIG_H macro). * modify the above headers so that they don't include config.h but contain the following instead: #ifndef __WINE_CONFIG_H # error You must include config.h to use this header #endif This will make sure that no-one forgets to include config.h when necessary. * modify all files that include the above headers so that they include config.h as the first header. This actually requires relatively little work. * do the same for port.h * debugtools.h also uses config.h because it needs NO_DEBUG_MSGS and NO_TRACE_MSGS. Modify configure.in so that these are put on the command line instead. They are not the of the same nature as the other macros anyway: they are completely system independent. Also this will make it possible to use the Wine debug macros in Winelib applications. Then debugtools.h will no longer need to #include config.h. This cuts the number of files causing trouble in about half. * there are 3 other files that #include config.h for no good reason that I can see: gdi.h, heap.h and thread.h. Remove the #includes from there. Anyway, winapi_check ensures that all the .c files that need config.h already include it directly. * maybe modify winapi_check to make sure that no header file includes 'config.h' (this would be the frosting on the cake) Doing this (except for port.h) I have a tree that builds on Solaris with no _FILE_OFFSET_BITS warning. So if the above sounds good, I have patches that are almost ready. -- François Gouget fgouget(a)codeweavers.com

1 0

Re: [Fwd: file inheritance]
by Alexandre Julliard Oct. 13, 2001

Oct. 13, 2001

eric pouech <eric.pouech(a)wanadoo.fr> writes: > this patch hasn't been committed yet > is there any issue Alexandre ? None, except that the patch conflicts with Mike's changes that I committed a few days ago. I was hoping you would submit an updated patch so I wouldn't have to merge it myself ;-) -- Alexandre Julliard julliard(a)winehq.com

1 0

Re: olepicture.c patch
by Francois Gouget Oct. 13, 2001

Oct. 13, 2001

Wouldn't this be better (do the trick inside the #ifdef HAVE_JPEGLIB_H): #ifdef HAVE_LIBJPEG /* This is a hack, so jpeglib.h does not redefine INT32 and the like*/ #define XMD_H #ifdef HAVE_JPEGLIB_H +# define UINT16 JPEG_UINT16 # include <jpeglib.h> +# undef UINT16 #endif #endif -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ If it stinks, it's chemistry. If it moves, it's biology. If it does not work, It's computer science.

1 0

Re: added missing CloseHandle
by eric pouech Oct. 13, 2001

Oct. 13, 2001

Kai Morich wrote: > > * dlls/shell32/shell.c > Kai Morich <mail(a)kai-morich.de> > InternalExtractIcon16 forgot to close file handle > > p.s. that's the reason why my last mail had no attachment, because the mailprogram couldn't open the still opened attachment file since we don't use the handle afterwards, it seems like OF_EXIST as a flag to OpenFile would be a much better way to handle this my 2 cents A+ -- --------------- Eric Pouech (http://perso.wanadoo.fr/eric.pouech/) "The future will be better tomorrow", Vice President Dan Quayle

1 0

Re: Don't TRACE NULL strings
by Dmitry Timoshkov Oct. 13, 2001

Oct. 13, 2001

"FranГois Gouget" <fgouget(a)codeweavers.com> wrote: > - TRACE("(%p %p %x)\n",pidl,szOut,uOutSize); > + TRACE("(%p %p %x)\n",pidl,debugstr_a(szOut),uOutSize); ^^ ^^^^^^^^^^^^^^^^^ Please recheck for pointer format vs. string argument mismatches. -- Dmitry.

1 0

Re: PATCH: Fix LoadCursor() under win9x
by Marcus Meissner Oct. 13, 2001

Oct. 13, 2001

On Fri, Oct 12, 2001 at 03:16:04PM +0200, Johan Gill wrote: > LoadCursor() should return 0 if we emulate win9x and the second parameter > is an invalid pointer. > + if (HIWORD(name)) > + if (GetVersion() & 0x80000000) /* win9x */ > + if (IsBadReadPtr(name, 4)) > + return 0; This code is unclean: It should not check for versions. It should not use IsBadReadPtr(), but IsBadStringPtrA(). It should not use IsBad* functions, but an exception handler. I would suggest the patch below. Ciao, Marcus Changelog: Handle bad pointer arguments to LoadImageA() (and functions calling LoadImageA) with an exception handler. Index: cursoricon.c =================================================================== RCS file: /home/wine/wine/windows/cursoricon.c,v retrieving revision 1.35 diff -u -r1.35 cursoricon.c --- cursoricon.c 2001/10/05 19:38:14 1.35 +++ cursoricon.c 2001/10/12 16:50:03 @@ -36,6 +36,7 @@ #include "wingdi.h" #include "wine/winbase16.h" #include "wine/winuser16.h" +#include "wine/exception.h" #include "heap.h" #include "palette.h" #include "bitmap.h" @@ -2174,14 +2175,29 @@ * FIXME: implementation lacks some features, see LR_ defines in winuser.h */ +/* filter for page-fault exceptions */ +static WINE_EXCEPTION_FILTER(page_fault) +{ + if (GetExceptionCode() == EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_EXECUTE_HANDLER; + return EXCEPTION_CONTINUE_SEARCH; +} + HANDLE WINAPI LoadImageA( HINSTANCE hinst, LPCSTR name, UINT type, INT desiredx, INT desiredy, UINT loadflags) { HANDLE res; LPWSTR u_name; - if (HIWORD(name)) u_name = HEAP_strdupAtoW(GetProcessHeap(), 0, name); - else u_name=(LPWSTR)name; + __TRY { + if (HIWORD(name)) u_name = HEAP_strdupAtoW(GetProcessHeap(), 0, name); + else u_name=(LPWSTR)name; + } + __EXCEPT(page_fault) { + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + __ENDTRY res = LoadImageW(hinst, u_name, type, desiredx, desiredy, loadflags); if (HIWORD(name)) HeapFree(GetProcessHeap(), 0, u_name); return res;

2 2

winemaker
by lawson_whitney＠juno.com Oct. 13, 2001

Oct. 13, 2001

Good day! I like winemaker, but it frustrates me. Given a standard wine installation (from source in /oof/wine, ./configure, make depend && make, make install) and a little winelib program with a handmade .spec file in subdirectory wlp, and the command winemaker --nosource-fix --console --nogenerated-specs \ --single-target toj2 wlp why does it make a configure script that can find -lwine, but can not find -lntdll - ? They are both in /usr/local/lib ... checking whether we can build a Linux dll... yes checking whether we need to define __i386__... no checking for g++ -fpermissive option... yes checking for g++ -fno-for-scope option... yes checking for windef.h header... /usr/local/include/wine checking for -lwine... checking for libntdll.so... configure: error: Could not find the Wine dlls (libntdll.so) The Winelib program is responsible to bring you this letter, but I had to hand-hack some scripts to get it to compile. Well, it may take me a while, but you or I will solve it. Regards, Lawson ---oof---

2 6

Re: PATCH: Fix LoadCursor() under win9x
by Alexandre Julliard Oct. 12, 2001

Oct. 12, 2001

Andreas Mohr <u4t5s0yq1001(a)sneakemail.com> writes: > Using your exception handler, LoadCursor() *always* returns 0 > on invalid parameter, which is WRONG in the NT 4 case > since it continues program execution without crashing. > > --> GetVersion() is required, check was (nearly) right IMHO. > Probably the best solution would be to use both an exception handler and > call GetVersion() on exception. Come on Andreas, we've been through this before. A version check must be added *only* when there is a real-life application that demonstrably depends on the version difference for proper behavior. So if you want the version check you'll have to show us an application that needs the crash in order to work right... -- Alexandre Julliard julliard(a)winehq.com

1 0

Re: winebuild and circular DLL dependencies
by Alexandre Julliard Oct. 12, 2001

Oct. 12, 2001

eric pouech <eric.pouech(a)wanadoo.fr> writes: > well, I think we need to differentiate two things: > - circular deps in normal imports > - circular refs of DLLs for delayed imports I consider that both are wrong. I think it's important to maintain proper layers and ensure that dll dependencies always go in one direction. Of course this is not always possible thanks to Microsoft design (or lack of it); but in the cases where we need to hack something I want it to be very explicit that something unusual is going on. If we make it so that any dll can easily import any other one, we'll quickly end up with an inextricable mess of cross-dll dependencies, simply because adding an import in the spec file for any dll that you may need is so much easier than having to use GetProcAddress and think about minimizing inter-dll calls because they are a pain to do. So there aren't really technical reasons we cannot do it, the issue is enforcing a structure by not making it too easy to do things the wrong way. -- Alexandre Julliard julliard(a)winehq.com

1 0