August 2004 - Wine-Devel - list.winehq.org

Re: DbgHelp: Fix for Includes with Relative Paths
by Eric Pouech Aug. 27, 2004

Aug. 27, 2004

Robert Shearman a écrit : > Hi, > > On startup, the debugger usually prompted me to type the path where > "/winternl.h" can be found. This was due to the path being stored > internally as "../../include/winternl.h", i.e. relative to the source > directory. This patch adds the ability to store the source directory and > then use it when encountering an include path of this type to get the > full path, which means no more prompting. > > Rob > > Changelog: > Store the current source directory and use it when encountering a > relative include so that the full path to it is stored instead. > Robert, normally currpath is supposed to be the directory where the file has been compiled. So, IMO currpath and srcpath duplicate the same stuff. Does it mean that you only get one N_SO per compilation unit ? (the code assumes two: the first one for the dir, the second for the path). A+

2 6

Re: closed reg keys. make sure CryptSetProviderEx doesnt delete the 'Type XXX' key
by Alexandre Julliard Aug. 27, 2004

Aug. 27, 2004

James Hawkins <truiken(a)gmail.com> writes: > Changelog > * make CryptSetProviderEx only delete the 'Name' value and not > delete the entire 'Type XXX' key when deleting the default provider. You are still leaking key handles... -- Alexandre Julliard julliard(a)winehq.org

1 0

Almost final epoll patch
by Shachar Shemesh Aug. 27, 2004

Aug. 27, 2004

Hi list, Attached is the epoll patch. All problems described in my previous patch sent should be solved here. The only reason I'm not sending this to wine-patches instead of here is that I have not yet had a chance to check it on my client's huge system, and so I cannot say for sure that all problems are truly and utterly resolved. In any case, I would welcome any and all comments about it. This patch is against CVS head, and should compile even if epoll.h is not available, as well as on old glibc's where epoll.h required stdint.h as a prerequisite, and the library's implementation did not have glibc. Shachar -- Shachar Shemesh Lingnu Open Source Consulting ltd. http://www.lingnu.com/ Index: configure =================================================================== RCS file: /home/sun/sources/cvs/wine/configure,v retrieving revision 1.592 diff -u -r1.592 configure --- configure 24 Aug 2004 21:00:15 -0000 1.592 +++ configure 27 Aug 2004 11:43:39 -0000 @@ -16207,6 +16207,7 @@ + for ac_header in \ arpa/inet.h \ arpa/nameser.h \ @@ -16269,6 +16270,7 @@ sys/msg.h \ sys/param.h \ sys/poll.h \ + sys/epoll.h \ sys/ptrace.h \ sys/reg.h \ sys/scsiio.h \ Index: configure.ac =================================================================== RCS file: /home/sun/sources/cvs/wine/configure.ac,v retrieving revision 1.303 diff -u -r1.303 configure.ac --- configure.ac 24 Aug 2004 21:00:15 -0000 1.303 +++ configure.ac 27 Aug 2004 11:42:58 -0000 @@ -1156,6 +1156,7 @@ sys/msg.h \ sys/param.h \ sys/poll.h \ + sys/epoll.h \ sys/ptrace.h \ sys/reg.h \ sys/scsiio.h \ Index: include/config.h.in =================================================================== RCS file: /home/sun/sources/cvs/wine/include/config.h.in,v retrieving revision 1.193 diff -u -r1.193 config.h.in --- include/config.h.in 4 Aug 2004 19:10:26 -0000 1.193 +++ include/config.h.in 27 Aug 2004 11:42:58 -0000 @@ -608,6 +608,9 @@ /* Define to 1 if you have the <sys/elf32.h> header file. */ #undef HAVE_SYS_ELF32_H +/* Define to 1 if you have the <sys/epoll.h> header file. */ +#undef HAVE_SYS_EPOLL_H + /* Define to 1 if you have the <sys/errno.h> header file. */ #undef HAVE_SYS_ERRNO_H Index: server/fd.c =================================================================== RCS file: /home/sun/sources/cvs/wine/server/fd.c,v retrieving revision 1.22 diff -u -r1.22 fd.c --- server/fd.c 18 Aug 2004 00:04:58 -0000 1.22 +++ server/fd.c 27 Aug 2004 11:47:12 -0000 @@ -33,6 +33,13 @@ #ifdef HAVE_SYS_POLL_H #include <sys/poll.h> #endif +#ifdef HAVE_SYS_EPOLL_H +#include <stdint.h> +#include <sys/epoll.h> +#endif +#ifdef HAVE_SYS_SYSCALL_H +#include <sys/syscall.h> +#endif #include <sys/stat.h> #include <sys/time.h> #include <sys/types.h> @@ -255,51 +262,173 @@ /****************************************************************/ -/* poll support */ +/* poll and epoll support */ -static struct fd **poll_users; /* users array */ -static struct pollfd *pollfd; /* poll fd array */ -static int nb_users; /* count of array entries actually in use */ +struct fdtrack { + enum { FDT_FREE, FDT_UNWATCHED, FDT_WATCHED, FDT_ERROR } state; + union { + int nextfree; + struct fd *fd; + } data; +}; + +/* Vars used by both */ static int active_users; /* current number of active users */ static int allocated_users; /* count of allocated entries in the array */ -static struct fd **freelist; /* list of free entries in the array */ +static struct fdtrack *poll_users; /* users array */ +static int freelist; /* list of free entries in the array. End of list marker is -1 */ + +/* Vars used by poll */ +static struct pollfd *pollfd; /* poll fd array */ +static int nb_users; /* Maximal index of array entry actually in use */ + +/* Vars used by epoll */ +static int epoll_fd=-1; /* File descriptor for the epoll */ +static const int EPOLL_SIZE=1024; /* "Maximal size" passed to epoll_create. This is an optimization, not a real maximum */ +static struct epoll_event *epoll_revents; /* Array for event results */ +static int epoll_revents_size; /* Number of elements in epoll_revents */ +static int in_epoll; /* Marks whether we are currently processing the epoll results */ +static int closependinglist; /* List of users in close pending state. End of list marker is -1 */ + +#if defined(HAVE_SYS_EPOLL_H) && (HAVE_SYS_EPOLL_H) +/* To prevent wine from not loading on non-epoll supporting platforms, + * we need to dynamically call the epoll functions */ +#ifndef __NR_epoll_create +#define __NR_epoll_create 254 +#endif + +#ifndef __NR_epoll_ctl +#define __NR_epoll_ctl 255 +#endif + +#ifndef __NR_epoll_wait +#define __NR_epoll_wait 256 +#endif + +static inline int _epoll_create( int size ) +{ + return syscall(__NR_epoll_create, size); +} + +static inline int _epoll_ctl(int epfd, int op, int fd, struct epoll_event *event) +{ + return syscall(__NR_epoll_ctl, epfd, op, fd, event); +} + +static inline int _epoll_wait(int epfd, struct epoll_event *events, int maxevents, int timeout) +{ + return syscall(__NR_epoll_wait, epfd, events, maxevents, timeout); +} +#else +/* epoll not available - make sure that the server still compiles and runs without it */ + +static inline int _epoll_create( int size ) +{ + errno=ENOSYS; + return -1; +} + +static inline int _epoll_ctl(int epfd, int op, int fd, struct epoll_event *event) +{ + errno=ENOSYS; + return -1; +} + +static inline int _epoll_wait(int epfd, struct epoll_event *events, int maxevents, int timeout) +{ + errno=ENOSYS; + return -1; +} + +/* The following definitions are incorrect ON PURPOSE. They are only meant to make code that will never be reached pass + * compilation. + */ +struct epoll_event { + unsigned int events; + struct { unsigned int u32; } data; +}; + +enum { EPOLL_CTL_DEL, EPOLL_CTL_MOD, EPOLL_CTL_ADD }; + +#endif + +/* Initialize a new area in the poll_users array to be free. + * Does not check input parameters - don't call with increment=0 */ +static void increase_poll_users_size( int increment ) +{ + struct fdtrack *new_poll_users=realloc(poll_users, sizeof(struct fdtrack)*(increment+allocated_users)); + struct pollfd *new_pollfd=(epoll_fd>=0)?NULL:realloc(pollfd, sizeof(struct pollfd)*(increment+allocated_users)); + + if( new_poll_users!=NULL && ( epoll_fd>=0 || new_pollfd!=NULL )) + { + int oldend=allocated_users; + int old_freelist=freelist; + + assert(old_freelist==-1); /* This funciton would never be called unless the free list is empty */ + + poll_users=new_poll_users; + pollfd=new_pollfd; + allocated_users+=increment; + freelist=oldend; + + while( increment>0 ) + { + poll_users[oldend].state=FDT_FREE; + poll_users[oldend].data.nextfree=oldend+1; + + if( epoll_fd<0 ) + { + pollfd[oldend].fd=-1; + pollfd[oldend].events=0; + pollfd[oldend].revents=0; + } + + oldend++; + increment--; + } + + assert(poll_users[oldend-1].state==FDT_FREE); /* If this assert fails, someone probably called us with a zero increment */ + poll_users[oldend-1].data.nextfree=old_freelist; + } + else if( new_poll_users!=NULL ) + { + /* The first realloc succeeded, the second one failed. Make sure the list is consistant. Not much else you can do */ + poll_users=new_poll_users; + } +} /* add a user in the poll array and return its index, or -1 on failure */ static int add_poll_user( struct fd *fd ) { int ret; - if (freelist) + + /* Find next free entry in list */ + if( freelist==-1 ) { - ret = freelist - poll_users; - freelist = (struct fd **)poll_users[ret]; + increase_poll_users_size(allocated_users); } - else + + ret=freelist; + + if( ret!=-1 ) { - if (nb_users == allocated_users) - { - struct fd **newusers; - struct pollfd *newpoll; - int new_count = allocated_users ? (allocated_users + allocated_users / 2) : 16; - if (!(newusers = realloc( poll_users, new_count * sizeof(*poll_users) ))) return -1; - if (!(newpoll = realloc( pollfd, new_count * sizeof(*pollfd) ))) - { - if (allocated_users) - poll_users = newusers; - else - free( newusers ); - return -1; - } - poll_users = newusers; - pollfd = newpoll; - allocated_users = new_count; - } - ret = nb_users++; - } - pollfd[ret].fd = -1; - pollfd[ret].events = 0; - pollfd[ret].revents = 0; - poll_users[ret] = fd; - active_users++; + assert(poll_users[ret].state==FDT_FREE); + assert(epoll_fd>=0 || pollfd[ret].fd==-1); + assert(epoll_fd>=0 || pollfd[ret].events==0); + assert(epoll_fd>=0 || pollfd[ret].revents==0); + + /* Mark the node as allocated */ + freelist=poll_users[ret].data.nextfree; + assert(poll_users[ret].state==FDT_FREE); + poll_users[ret].state=FDT_UNWATCHED; + poll_users[ret].data.fd = fd; + active_users++; + + if( epoll_fd<0 && ret>=nb_users ) + nb_users=ret+1; + } + /* If the above "if" fails, it means there is no memory for the new entry */ + return ret; } @@ -307,17 +436,68 @@ static void remove_poll_user( struct fd *fd, int user ) { assert( user >= 0 ); - assert( poll_users[user] == fd ); - pollfd[user].fd = -1; - pollfd[user].events = 0; - pollfd[user].revents = 0; - poll_users[user] = (struct fd *)freelist; - freelist = &poll_users[user]; + assert( poll_users[user].data.fd==fd && fd->poll_index==user ); + assert( poll_users[user].state!=FDT_WATCHED || fd->unix_fd!=-1 ); + /* If this fails it means that there is a file descriptor still being watched by us, that is no longer recorded in the fd struct. + * This is not a problem for poll, and is not a problem for epoll IF the file descriptor was indeed closed. */ + + if( epoll_fd>=0 ) + { + if( fd->unix_fd!=-1 ) + { + struct epoll_event event; /* This struct needn't actually contain anything meaningful, as we are merely calling DEL */ + _epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fd->unix_fd, &event); + /* XXX - May need to check return code, but there is really nothing to do on failure. + * In any case, failing with errno=ENOENT is ok. + */ + } + } + else + { + pollfd[user].fd = -1; + pollfd[user].events = 0; + pollfd[user].revents = 0; + } + + /* Mark the entry for reuse */ + poll_users[user].state=FDT_FREE; + if( in_epoll ) + { + /* This junction prevents a race where an epoll returned several fds, and this fd is one of them. Before epoll gets to handling + * this fd, however, a previous fd asks to release this fd. As a result, epoll might call a handler on a released fd. epoll checks + * the state to avoid this race. + * However, it may still happen that the same epoll handling loop may release this fd, and then reallocate for a totally different + * purpose. This will foil the state checking safty measure. + * + * To prevent this, fds freed when epoll is still processing results are + * put on a pending list, and are only added to the free list after epoll finishes processing. + * + * Notice when changing this comment - there are two comments in main_loop that refer to this one. Make sure these are updated as well. + */ + poll_users[user].data.nextfree=closependinglist; + closependinglist=user; + } + else + { + poll_users[user].data.nextfree=freelist; + freelist=user; + } active_users--; } +/* initialize the fd related state */ +void init_fd() +{ + /* If this function fails, epoll_fd will be -1, which will signal + * the rest of the functions to use poll instead */ + epoll_fd=_epoll_create(EPOLL_SIZE); + + freelist=-1; + closependinglist=-1; + increase_poll_users_size(32); +} -/* server main poll() loop */ +/* server main poll() or epoll() loop */ void main_loop(void) { int ret; @@ -341,16 +521,79 @@ } if (!active_users) break; /* last user removed by a timeout */ } - ret = poll( pollfd, nb_users, diff ); - if (ret > 0) + if( epoll_fd>=0 ) { + /* if the event handlers added a lot of new events, we may want to increase the returned + * events space */ + if( epoll_revents_size<active_users ) + { + /* If epoll_revents_size is below active_users, there is a (very slim) potential for starvation. + * It is not, strictly speaking, an error though. As such, if the allocation fails, we just go on + * with the smaller array size. */ + struct epoll_event *new_revents=malloc(sizeof(struct epoll_event)*active_users); + if( new_revents!=NULL ) + { + free(epoll_revents); + epoll_revents=new_revents; + epoll_revents_size=active_users; + } + } + + in_epoll=1; + ret = _epoll_wait(epoll_fd, epoll_revents, epoll_revents_size, diff ); + if( ret>=0 ) + { + while( (ret--)>0 ) + { + /* This "if" is preventing a race. The race is documented in a comment inside the "remove_poll_user" function */ + if( poll_users[epoll_revents[ret].data.u32].state==FDT_WATCHED ) + fd_poll_event(poll_users[epoll_revents[ret].data.u32].data.fd, epoll_revents[ret].events); + } + } + else + { + if( errno!=EINTR ) + perror("wineserver: main_loop epoll"); + else if( debug_level ) + perror("wineserver: main_loop epoll (nothing to worry about)"); + } + in_epoll=0; + + /* Free the close pending users. + * Link the current free list to the end of the pending list, and transfer to the free list. + * The race prevented by these actions is documented in a comment in the "remove_poll_user" function + */ + if(closependinglist!=-1) + { + int nextfree, currentfree; + + currentfree=closependinglist; + + assert(poll_users[currentfree].state==FDT_FREE); + + while( (nextfree=poll_users[currentfree].data.nextfree)!=-1 ) + { + assert(poll_users[nextfree].state==FDT_FREE); + currentfree=nextfree; + } + + poll_users[currentfree].data.nextfree=freelist; + freelist=closependinglist; + closependinglist=-1; + } + } + else + { + /* No epoll support - use poll */ int i; - for (i = 0; i < nb_users; i++) + + ret = poll( pollfd, nb_users, diff ); + for (i = 0; ret>0 && i < nb_users; i++) { if (pollfd[i].revents) { - fd_poll_event( poll_users[i], pollfd[i].revents ); - if (!--ret) break; + fd_poll_event( poll_users[i].data.fd, pollfd[i].revents ); + --ret; } } } @@ -835,18 +1078,48 @@ /* set the events that select waits for on this fd */ void set_fd_events( struct fd *fd, int events ) { - int user = fd->poll_index; - assert( poll_users[user] == fd ); - if (events == -1) /* stop waiting on this fd completely */ + int user=fd->poll_index; + assert(poll_users[user].state!=FDT_FREE); + assert( poll_users[user].data.fd == fd ); + + if( epoll_fd>=0 ) { - pollfd[user].fd = -1; - pollfd[user].events = POLLERR; - pollfd[user].revents = 0; + struct epoll_event event; + event.events=events; + event.data.u32=user; + + assert(fd->unix_fd>=0); + + if( events != -1 && poll_users[user].state!=FDT_ERROR ) + { + int epoll_res; + + epoll_res=_epoll_ctl( epoll_fd, (poll_users[user].state==FDT_WATCHED)?EPOLL_CTL_MOD:EPOLL_CTL_ADD, fd->unix_fd, &event ); + /* FIXME: Need to handle failure here */ + poll_users[user].state=FDT_WATCHED; + } + else if( events==-1 ) + { + /* flag fd as erred */ + _epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fd->unix_fd, &event ); + poll_users[user].state=FDT_ERROR; + } } - else if (pollfd[user].fd != -1 || !pollfd[user].events) + else { - pollfd[user].fd = fd->unix_fd; - pollfd[user].events = events; + if (events == -1) /* stop waiting on this fd completely */ + { + pollfd[user].fd = -1; + pollfd[user].events = POLLERR; + pollfd[user].revents = 0; + poll_users[user].state=FDT_ERROR; + } + else if (poll_users[user].state!=FDT_ERROR) + { + pollfd[user].fd = fd->unix_fd; + pollfd[user].events = events; + poll_users[user].state = FDT_WATCHED; + } } } Index: server/file.h =================================================================== RCS file: /home/sun/sources/cvs/wine/server/file.h,v retrieving revision 1.17 diff -u -r1.17 file.h --- server/file.h 18 Aug 2004 00:04:58 -0000 1.17 +++ server/file.h 27 Aug 2004 11:42:58 -0000 @@ -44,6 +44,7 @@ /* file descriptor functions */ +extern void init_fd(); extern struct fd *alloc_fd( const struct fd_ops *fd_user_ops, struct object *user ); extern struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode, unsigned int access, unsigned int sharing, unsigned int options ); Index: server/main.c =================================================================== RCS file: /home/sun/sources/cvs/wine/server/main.c,v retrieving revision 1.31 diff -u -r1.31 main.c --- server/main.c 26 Mar 2003 01:32:18 -0000 1.31 +++ server/main.c 27 Aug 2004 11:42:58 -0000 @@ -122,6 +122,7 @@ signal( SIGTERM, sigterm_handler ); signal( SIGABRT, sigterm_handler ); + init_fd(); sock_init(); open_master_socket(); sync_namespace = create_namespace( 37, TRUE );

1 0

Fwd: Re: make CryptSetProviderEx update the provider Name and TypeName to registry
by James Hawkins Aug. 27, 2004

Aug. 27, 2004

---------- Forwarded message ---------- From: James Hawkins <truiken(a)gmail.com> Date: Fri, 27 Aug 2004 08:18:48 -0400 Subject: Re: Re: make CryptSetProviderEx update the provider Name and TypeName to registry To: Jakob Eriksson <jakob(a)vmlinux.org> I sent in a patch earlier that added tests for CryptSetProviderEx. It's been committed. On Fri, 27 Aug 2004 11:39:55 +0200, Jakob Eriksson <jakob(a)vmlinux.org> wrote: > > Can you add this test code to the Wine regression suite? > > > > > James Hawkins wrote: > > >I just ran a test programming in windows to see how the registry is > >changed by called CryptSetProviderEx with the CRYPT_DELETE_DEFAULT > >flag. When the CRYPT_DELETE_DEFAULT flag is used, the Name value of > >the Type XXX key is removed, but not the TypeName, which solves the > >problem of not being able to get to the TypeName. I will rewrite this > >patch with the new behavior and send it shortly. > > > >On Fri, 20 Aug 2004 13:11:12 -0700, James Hawkins <truiken(a)gmail.com> wrote: > > > > > >>I'm working on a test app that I hope wil shine some light on how > >>windows gets the Name and TypeName values back in the registry after > >>they've been deleted. The problem is that the only instance of the > >>Name and TypeName data is in the DllRegisterServer function of the > >>provider dll. I couldn't think of any other way to get to that data > >>to get it in the registry, but I will be testing this in windows. > >>What im thinking is that windows might not even delete the key at all. > >> If you have any suggestions it would help a lot. > >> > >> > >> > >>On Fri, 20 Aug 2004 12:48:07 -0700, Alexandre Julliard > >><julliard(a)winehq.org> wrote: > >> > >> > >>>James Hawkins <truiken(a)gmail.com> writes: > >>> > >>> > >>> > >>>>>Changelog > >>>>> * Call provider.dll's DllRegisterServer from CryptSetProviderEx to > >>>>>update the Name and TypeName values in the registry properly when > >>>>>setting the default provider > >>>>> > >>>>> > >>>It doesn't seem right to call DllRegisterServer from there, IMO it > >>>should only be called when the dll is installed. Do you have evidence > >>>that Windows does it this way? > >>> > >>>-- > >>>Alexandre Julliard > >>>julliard(a)winehq.org > >>> > >>> > >>> > >>-- > >>James Hawkins > >> > >> > >> > > > > > > > > > > -- James Hawkins -- James Hawkins

1 0

RE: RICHEDIT: use buffers rather than linked lists for input and out buffers
by Ge van Geldorp Aug. 27, 2004

Aug. 27, 2004

> From: Mike McCormack > > This patch writes the output of the RTF parser into the edit > control in > 1024 byte chunks, and reads the input via a char[1024] buffer, rather > than a linked list of single characters. Not that it should matter very much, but the MS control always seems to read the input in 4096 byte chunks. Maybe you could enlarge InputBuffer from 0x400 to 0x1000? Ge van Geldorp.

1 0

Remote operations -- need for advice
by Alexander Yaworsky Aug. 27, 2004

Aug. 27, 2004

I'm thinking how to implement CreateRemoteThread and besides fix memory management functions. The complete (afaik) list includes: RtlCreateUserThread NtAllocateVirtualMemory NtFreeVirtualMemory NtProtectVirtualMemory NtQueryVirtualMemory NtLockVirtualMemory (do nothing?) NtUnlockVirtualMemory (do nothing?) NtFlushVirtualMemory NtMapViewOfSection NtUnmapViewOfSection Suggested implementation. Let's add two groups of handlers to the wineserver, something like remote_operation_xxxx and remote_operation_xxxx_complete. remote_operation should do the following: 1) suspend_for_ptrace(); 2) inject a piece of code into required process and start its execution; 3) resume_after_ptrace(); 4) place calling thread into suspended state (or into some wait state?) remote_operation_complete should prepare reply and resume thread suspended by remote_operation. Injected code should call required function and then remote_operation_complete in context of required process. The question is: how to correctly get address of function? imho possible solutions are: 1) assume ntdll loaded at the same address for all processes -- unreliable; 2) get dll base address from per-process dll list and parse ELF by hand -- too complicated (?); 3) pass relative offsets and add them later to the ntdll's base address -- unreliable a bit: ntdll may be replaced, although, it is unlikely; 4) extend the struct process (server/process.h) and the request init_process with pointers to required functions -- most reliable and simple but looks ugly. What do you think?

6 11

Localizing shell32/shv_item_cmenu.c:ISvItemCm_fnQueryContextMenu
by Tobias Burnus Aug. 27, 2004

Aug. 27, 2004

Hello, if I right click in the file open dialog, select a file and right click, I get a context menu with English menu items. Expected: I'll get a localized menu (e.g. in German). Any idea how to get a localized version? From dlls/shell32/shv_item_cmenu.c ------------------------------------ * ISvItemCm_fnQueryContextMenu() */ static HRESULT WINAPI ISvItemCm_fnQueryContextMenu( [...] _InsertMenuItem(hmenu, indexMenu++, TRUE, FCIDM_SHVIEW_OPEN, MFT_STRING, "&Open", MFS_ENABLED); _InsertMenuItem(hmenu, indexMenu++, TRUE, FCIDM_SHVIEW_EXPLORE, MFT_STRING, "&Explore", MFS_ENABLED|MFS_DEFAULT); } [...] ------------------------------------ Tobias PS: the _InsertMenuItem function, defined in the same file, uses InsertMenuItemA, one might want to change this to InsertMenuItemW.

2 1

Re: RESEND make sure CryptSetProviderEx doesnt delete the 'Type XXX' key
by Alexandre Julliard Aug. 27, 2004

Aug. 27, 2004

James Hawkins <truiken(a)gmail.com> writes: > This is a resend of this patch...it's been through a couple times. If > there is anything keeping the patch from being committed, please let > me know. Thanks. One problem I see is that you never close the key handles that you open. -- Alexandre Julliard julliard(a)winehq.org

1 0

Something broke winetest?
by Paul Millar Aug. 26, 2004

Aug. 26, 2004

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Looks like something broke building cross-compiled tests last night. I now get: [paulm(a)quisquiliae tests]$ make i686-mingw32msvc-gcc dpa.o imagelist.o subclass.o tab.o testlist.o -o comctl32_test.exe -lcomctl32 -luser32 -lgdi32 imagelist.o(.text+0x1077): In function `DoTest3': /home/paulm/Testing/wine-cross-source/dlls/comctl32/tests/imagelist.c:265: undefined reference to `ImageList_DrawIndirect(a)4' imagelist.o(.text+0x10b7):/home/paulm/Testing/wine-cross-source/dlls/comctl32/tests/imagelist.c:267: undefined reference to `ImageList_DrawIndirect(a)4' imagelist.o(.text+0x10f6):/home/paulm/Testing/wine-cross-source/dlls/comctl32/tests/imagelist.c:269: undefined reference to `ImageList_DrawIndirect(a)4' imagelist.o(.text+0x1132):/home/paulm/Testing/wine-cross-source/dlls/comctl32/tests/imagelist.c:271: undefined reference to `ImageList_DrawIndirect(a)4' imagelist.o(.text+0x1186):/home/paulm/Testing/wine-cross-source/dlls/comctl32/tests/imagelist.c:277: undefined reference to `ImageList_DrawIndirect(a)4' imagelist.o(.text+0x11ba):/home/paulm/Testing/wine-cross-source/dlls/comctl32/tests/imagelist.c:279: more undefined references to `ImageList_DrawIndirect(a)4' follow distcc[21076] ERROR: compile on localhost failed make: *** [comctl32_test.exe] Error 1 [paulm(a)quisquiliae tests]$ Which causes comctl32_test.exe not to be built, so the build of winetest.exe fails. Do others get this too? Cheers, Paul. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.4 (GNU/Linux) iD8DBQFBLIJM/9JwS78PA+kRAjN+AKCUTBxmUZlJQnPvUKZWuM2n8rGBVgCaA7Wz qE5hC6AGHcijndK6umrZygQ= =vivD -----END PGP SIGNATURE-----

3 3

Non-perfect epoll patch
by Shachar Shemesh Aug. 26, 2004

Aug. 26, 2004

Hi all, Attached is a non-perfect patch for review. This is a migration of the wineserver to use epoll instead of poll (if it's available). current known issue with this patch: 1. Will not compile if HAVE_SYS_EPOLL_H is not 1 (i.e. - won't compile if epoll not available at compile time) 2. Segfaults on wine exit. 3. Lots of debug asserts. Comments welcome. Shachar -- Shachar Shemesh Lingnu Open Source Consulting ltd. http://www.lingnu.com/ Index: configure =================================================================== RCS file: /home/sun/sources/cvs/wine/configure,v retrieving revision 1.573 diff -u -r1.573 configure --- configure 17 Jul 2004 00:52:37 -0000 1.573 +++ configure 24 Aug 2004 12:03:32 -0000 @@ -6727,6 +6727,79 @@ fi +echo "$as_me:$LINENO: checking for epoll_create in -lepoll" >&5 +echo $ECHO_N "checking for epoll_create in -lepoll... $ECHO_C" >&6 +if test "${ac_cv_lib_epoll_epoll_create+set}" = set; then + echo $ECHO_N "(cached) $ECHO_C" >&6 +else + ac_check_lib_save_LIBS=$LIBS +LIBS="-lepoll $LIBS" +cat >conftest.$ac_ext <<_ACEOF +/* confdefs.h. */ +_ACEOF +cat confdefs.h >>conftest.$ac_ext +cat >>conftest.$ac_ext <<_ACEOF +/* end confdefs.h. */ + +/* Override any gcc2 internal prototype to avoid an error. */ +#ifdef __cplusplus +extern "C" +#endif +/* We use char because int might match the return type of a gcc2 + builtin and then its argument prototype would still apply. */ +char epoll_create (); +int +main () +{ +epoll_create (); + ; + return 0; +} +_ACEOF +rm -f conftest.$ac_objext conftest$ac_exeext +if { (eval echo "$as_me:$LINENO: \"$ac_link\"") >&5 + (eval $ac_link) 2>conftest.er1 + ac_status=$? + grep -v '^ *+' conftest.er1 >conftest.err + rm -f conftest.er1 + cat conftest.err >&5 + echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); } && + { ac_try='test -z "$ac_c_werror_flag" || test ! -s conftest.err' + { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5 + (eval $ac_try) 2>&5 + ac_status=$? + echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); }; } && + { ac_try='test -s conftest$ac_exeext' + { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5 + (eval $ac_try) 2>&5 + ac_status=$? + echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); }; }; then + ac_cv_lib_epoll_epoll_create=yes +else + echo "$as_me: failed program was:" >&5 +sed 's/^/| /' conftest.$ac_ext >&5 + +ac_cv_lib_epoll_epoll_create=no +fi +rm -f conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +LIBS=$ac_check_lib_save_LIBS +fi +echo "$as_me:$LINENO: result: $ac_cv_lib_epoll_epoll_create" >&5 +echo "${ECHO_T}$ac_cv_lib_epoll_epoll_create" >&6 +if test $ac_cv_lib_epoll_epoll_create = yes; then + cat >>confdefs.h <<_ACEOF +#define HAVE_LIBEPOLL 1 +_ACEOF + + LIBS="-lepoll $LIBS" + +fi + + echo "$as_me:$LINENO: checking for res_9_init in -lresolv" >&5 echo $ECHO_N "checking for res_9_init in -lresolv... $ECHO_C" >&6 if test "${ac_cv_lib_resolv_res_9_init+set}" = set; then @@ -16190,6 +16263,7 @@ + for ac_func in \ _lwp_create \ _lwp_self \ @@ -16202,6 +16276,7 @@ _vsnprintf \ chsize \ clone \ + epoll_create \ finite \ fpclass \ fstatfs \ @@ -16435,6 +16510,7 @@ + for ac_header in \ arpa/inet.h \ arpa/nameser.h \ @@ -16495,6 +16571,7 @@ sys/msg.h \ sys/param.h \ sys/poll.h \ + sys/epoll.h \ sys/ptrace.h \ sys/reg.h \ sys/scsiio.h \ Index: configure.ac =================================================================== RCS file: /home/sun/sources/cvs/wine/configure.ac,v retrieving revision 1.285 diff -u -r1.285 configure.ac --- configure.ac 6 Jul 2004 21:01:19 -0000 1.285 +++ configure.ac 24 Aug 2004 12:01:22 -0000 @@ -140,6 +140,8 @@ AC_CHECK_LIB(xpg4,_xpg4_setrunelocale) dnl Check for -lpoll for Mac OS X/Darwin AC_CHECK_LIB(poll,poll) +dnl Check for -lepoll +AC_CHECK_LIB(epoll,epoll_create) dnl Check for -lresolv for Mac OS X/Darwin AC_CHECK_LIB(resolv,res_9_init) dnl Check for -lpthread @@ -1036,6 +1038,7 @@ _vsnprintf \ chsize \ clone \ + epoll_create \ finite \ fpclass \ fstatfs \ @@ -1146,6 +1149,7 @@ sys/msg.h \ sys/param.h \ sys/poll.h \ + sys/epoll.h \ sys/ptrace.h \ sys/reg.h \ sys/scsiio.h \ Index: include/config.h.in =================================================================== RCS file: /home/sun/sources/cvs/wine/include/config.h.in,v retrieving revision 1.190 diff -u -r1.190 config.h.in --- include/config.h.in 18 Jun 2004 19:36:26 -0000 1.190 +++ include/config.h.in 24 Aug 2004 12:05:42 -0000 @@ -80,6 +80,9 @@ /* Define to 1 if you have the <elf.h> header file. */ #undef HAVE_ELF_H +/* Define to 1 if you have the `epoll_create' function. */ +#undef HAVE_EPOLL_CREATE + /* Define to 1 if you have the `finite' function. */ #undef HAVE_FINITE @@ -248,6 +251,9 @@ /* Define if you have the curses library (-lcurses) */ #undef HAVE_LIBCURSES +/* Define to 1 if you have the `epoll' library (-lepoll). */ +#undef HAVE_LIBEPOLL + /* Define if you have libgif/libungif including devel headers */ #undef HAVE_LIBGIF @@ -608,6 +614,9 @@ /* Define to 1 if you have the <sys/elf32.h> header file. */ #undef HAVE_SYS_ELF32_H +/* Define to 1 if you have the <sys/epoll.h> header file. */ +#undef HAVE_SYS_EPOLL_H + /* Define to 1 if you have the <sys/errno.h> header file. */ #undef HAVE_SYS_ERRNO_H Index: server/fd.c =================================================================== RCS file: /home/sun/sources/cvs/wine/server/fd.c,v retrieving revision 1.21 diff -u -r1.21 fd.c --- server/fd.c 22 May 2004 03:15:04 -0000 1.21 +++ server/fd.c 24 Aug 2004 21:20:53 -0000 @@ -33,6 +33,12 @@ #ifdef HAVE_SYS_POLL_H #include <sys/poll.h> #endif +#ifdef HAVE_SYS_EPOLL_H +#include <sys/epoll.h> +#endif +#ifdef HAVE_SYS_SYSCALL_H +#include <sys/syscall.h> +#endif #include <sys/stat.h> #include <sys/time.h> #include <sys/types.h> @@ -48,6 +54,9 @@ #include "winreg.h" #include "winternl.h" +#undef NDEBUG +#include <assert.h> + /* Because of the stupid Posix locking semantics, we need to keep * track of all file descriptors referencing a given file, and not * close a single one until all the locks are gone (sigh). @@ -255,51 +264,107 @@ /****************************************************************/ -/* poll support */ +/* poll and epoll support */ + +/* Vars used by both */ +static int active_users; /* current number of active users */ +/* Vars used by poll */ static struct fd **poll_users; /* users array */ static struct pollfd *pollfd; /* poll fd array */ static int nb_users; /* count of array entries actually in use */ -static int active_users; /* current number of active users */ static int allocated_users; /* count of allocated entries in the array */ static struct fd **freelist; /* list of free entries in the array */ +/* Vars used by epoll */ +static int epoll_fd=-1; /* File descriptor for the epoll */ +static const int EPOLL_SIZE=1024; /* "Maximal size" passed to epoll_create. This is an optimization, not a real maximum */ +static struct epoll_event *epoll_revents; /* Array for event results */ +static int epoll_revents_size; /* Number of elements in epoll_revents */ + +#if defined(HAVE_EPOLL_CREATE) && defined(HAVE_SYS_SYSCALL_H) +/* To prevent wine from not loading on non-epoll supporting platforms, + * we need to dynamically call the epoll functions */ +static inline int _epoll_create( int size ) +{ + return syscall(__NR_epoll_create, size); +} + +static inline int _epoll_ctl(int epfd, int op, int fd, struct epoll_event *event) +{ + return syscall(__NR_epoll_ctl, epfd, op, fd, event); +} + +static inline int _epoll_wait(int epfd, struct epoll_event *events, int maxevents, int timeout) +{ + return syscall(__NR_epoll_wait, epfd, events, maxevents, timeout); +} +#else +static inline int _epoll_create( int size ) +{ + errno=ENOSYS; + return -1; +} + +static inline int _epoll_ctl(int epfd, int op, int fd, struct epoll_event *event) +{ + errno=ENOSYS; + return -1; +} + +static inline int _epoll_wait(int epfd, struct epoll_event *events, int maxevents, int timeout) +{ + errno=ENOSYS; + return -1; +} +#endif + + /* add a user in the poll array and return its index, or -1 on failure */ static int add_poll_user( struct fd *fd ) { int ret; - if (freelist) + if( epoll_fd>=0 ) /* We are using the epoll interface */ { - ret = freelist - poll_users; - freelist = (struct fd **)poll_users[ret]; + /* No poll array when using epoll - always return 1 */ + ret=1; } else - { - if (nb_users == allocated_users) + { /* We are using the poll interface */ + if (freelist) { - struct fd **newusers; - struct pollfd *newpoll; - int new_count = allocated_users ? (allocated_users + allocated_users / 2) : 16; - if (!(newusers = realloc( poll_users, new_count * sizeof(*poll_users) ))) return -1; - if (!(newpoll = realloc( pollfd, new_count * sizeof(*pollfd) ))) + ret = freelist - poll_users; + freelist = (struct fd **)poll_users[ret]; + } + else + { + if (nb_users == allocated_users) { - if (allocated_users) - poll_users = newusers; - else - free( newusers ); - return -1; + struct fd **newusers; + struct pollfd *newpoll; + int new_count = allocated_users ? (allocated_users + allocated_users / 2) : 16; + if (!(newusers = realloc( poll_users, new_count * sizeof(*poll_users) ))) return -1; + if (!(newpoll = realloc( pollfd, new_count * sizeof(*pollfd) ))) + { + if (allocated_users) + poll_users = newusers; + else + free( newusers ); + return -1; + } + poll_users = newusers; + pollfd = newpoll; + allocated_users = new_count; } - poll_users = newusers; - pollfd = newpoll; - allocated_users = new_count; - } - ret = nb_users++; - } - pollfd[ret].fd = -1; - pollfd[ret].events = 0; - pollfd[ret].revents = 0; - poll_users[ret] = fd; + ret = nb_users++; + } + pollfd[ret].fd = -1; + pollfd[ret].events = 0; + pollfd[ret].revents = 0; + poll_users[ret] = fd; + } active_users++; + return ret; } @@ -307,15 +372,37 @@ static void remove_poll_user( struct fd *fd, int user ) { assert( user >= 0 ); - assert( poll_users[user] == fd ); - pollfd[user].fd = -1; - pollfd[user].events = 0; - pollfd[user].revents = 0; - poll_users[user] = (struct fd *)freelist; - freelist = &poll_users[user]; + if( epoll_fd>=0 ) + { + if( fd->unix_fd!=-1 ) + { + struct epoll_event event; /* This struct needn't actually contain anything meaningful, as we are merely calling DEL */ + if(!(_epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fd->unix_fd, &event)==0 || errno==ENOENT)) + { + int err; + err=errno; + } + } + } + else + { + assert( poll_users[user] == fd ); + pollfd[user].fd = -1; + pollfd[user].events = 0; + pollfd[user].revents = 0; + poll_users[user] = (struct fd *)freelist; + freelist = &poll_users[user]; + } active_users--; } +/* initialize the fd related state */ +void init_fd() +{ + /* If this function fails, epoll_fd will be -1, which will signal + * the rest of the functions to use poll instead */ + epoll_fd=_epoll_create(EPOLL_SIZE); +} /* server main poll() loop */ void main_loop(void) @@ -341,16 +428,44 @@ } if (!active_users) break; /* last user removed by a timeout */ } - ret = poll( pollfd, nb_users, diff ); - if (ret > 0) + if( epoll_fd>=0 ) { - int i; - for (i = 0; i < nb_users; i++) + /* if the event handlers added a lot of new events, we may want to increase the returned + * events space */ + if( epoll_revents_size<active_users ) { - if (pollfd[i].revents) + /* If epoll_revents_size is below active_users, there is a (very slim) potential for starvation. + * It is not, strictly speaking, an error though. As such, if the allocation fails, we just go on + * with the smaller array size. */ + struct epoll_event *new_revents=malloc(sizeof(struct epoll_event)*active_users); + if( new_revents!=NULL ) { - fd_poll_event( poll_users[i], pollfd[i].revents ); - if (!--ret) break; + free(epoll_revents); + epoll_revents=new_revents; + epoll_revents_size=active_users; + } + } + + ret = epoll_wait(epoll_fd, epoll_revents, epoll_revents_size, diff ); + while( (ret--)>0 ) + { + fd_poll_event((struct fd *)epoll_revents[ret].data.ptr, epoll_revents[ret].events); + } + } + else + { + /* No epoll support - use poll */ + ret = poll( pollfd, nb_users, diff ); + if (ret > 0) + { + int i; + for (i = 0; i < nb_users; i++) + { + if (pollfd[i].revents) + { + fd_poll_event( poll_users[i], pollfd[i].revents ); + if (!--ret) break; + } } } } @@ -835,18 +950,46 @@ /* set the events that select waits for on this fd */ void set_fd_events( struct fd *fd, int events ) { - int user = fd->poll_index; - assert( poll_users[user] == fd ); - if (events == -1) /* stop waiting on this fd completely */ + if( epoll_fd>=0 ) { - pollfd[user].fd = -1; - pollfd[user].events = POLLERR; - pollfd[user].revents = 0; + struct epoll_event event; + event.events=events; + event.data.ptr=fd; + + assert(fd->unix_fd>=0); + + if( events != -1 ) + { + int epoll_res; + if( (epoll_res=_epoll_ctl(epoll_fd, EPOLL_CTL_MOD, fd->unix_fd, &event )!=0) && errno==ENOENT ) + { + /* For one reason or another this file descriptor isn't being watched at the moment. Just add it */ + assert(_epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd->unix_fd, &event )==0); + } + else + assert(epoll_res==0); + } + else + { + /* Stop waiting on this fd completely */ + assert(_epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fd->unix_fd, &event )==0); + } } - else if (pollfd[user].fd != -1 || !pollfd[user].events) + else { - pollfd[user].fd = fd->unix_fd; - pollfd[user].events = events; + int user = fd->poll_index; + assert( poll_users[user] == fd ); + if (events == -1) /* stop waiting on this fd completely */ + { + pollfd[user].fd = -1; + pollfd[user].events = POLLERR; + pollfd[user].revents = 0; + } + else if (pollfd[user].fd != -1 || !pollfd[user].events) + { + pollfd[user].fd = fd->unix_fd; + pollfd[user].events = events; + } } } Index: server/file.h =================================================================== RCS file: /home/sun/sources/cvs/wine/server/file.h,v retrieving revision 1.16 diff -u -r1.16 file.h --- server/file.h 16 Apr 2004 04:31:35 -0000 1.16 +++ server/file.h 24 Aug 2004 13:34:21 -0000 @@ -44,6 +44,7 @@ /* file descriptor functions */ +extern void init_fd(); extern struct fd *alloc_fd( const struct fd_ops *fd_user_ops, struct object *user ); extern struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode, unsigned int access, unsigned int sharing, unsigned int options ); Index: server/main.c =================================================================== RCS file: /home/sun/sources/cvs/wine/server/main.c,v retrieving revision 1.31 diff -u -r1.31 main.c --- server/main.c 26 Mar 2003 01:32:18 -0000 1.31 +++ server/main.c 24 Aug 2004 15:49:33 -0000 @@ -122,6 +122,7 @@ signal( SIGTERM, sigterm_handler ); signal( SIGABRT, sigterm_handler ); + init_fd(); sock_init(); open_master_socket(); sync_namespace = create_namespace( 37, TRUE );

2 3