patch: dup.diff Get rid of superfluous dup() and close() calls. Modified files: dlls/kernel/comm.c | 25 ------------------ dlls/ntdll/file.c | 2 - dlls/ntdll/virtual.c | 5 --- dlls/winsock/socket.c | 68 +------------------------------------------------- files/file.c | 15 ----------- files/smb.c | 1 include/async.h | 1 scheduler/client.c | 1 Comment: The dup() in wine_server_handle_to_fd() requires dozens of close() calls spread over the wine code. I think this is confusing and error-prone (close() calls are easily forgotten, leading to rapid fd leak - actually, when looking through the code, I found a number of functions where I had expected a close(), but it was lacking). Moreover, I can't see why that dup() is necessary (any more). If there are still some points in the code where the dup() is really required, I strongly recommend to insert explicit dup()s in the respective functions after calling wine_server_handle_to_fd(). I have carried out a number of IO intensive tests with no problems. I'd like people to test the COMM port stuff, though, and watch the number of file descriptors. Other IO- and Network-bound tests would be very welcome. If this is absolutely dumb, please tell me why. Forwarding to wine-devel, 'cause it's pretty intrusive. Index: dlls/kernel/comm.c =================================================================== RCS file: /home/wine/wine/dlls/kernel/comm.c,v retrieving revision 1.68 diff -u -r1.68 comm.c --- dlls/kernel/comm.c 25 Oct 2002 21:02:30 -0000 1.68 +++ dlls/kernel/comm.c 24 Jan 2003 14:13:32 -0000 @@ -516,7 +516,6 @@ return FALSE; } result = ioctl(fd,TIOCSBRK,0); - close(fd); if (result ==-1) { TRACE("ioctl failed\n"); @@ -556,7 +555,6 @@ return FALSE; } result = ioctl(fd,TIOCCBRK,0); - close(fd); if (result ==-1) { TRACE("ioctl failed\n"); @@ -598,7 +596,6 @@ if (tcgetattr(fd,&port) == -1) { COMM_SetCommError(handle,CE_IOE); - close(fd); return FALSE; } @@ -670,7 +667,6 @@ if (!direct) if (tcsetattr(fd, TCSADRAIN, &port) == -1) { - close(fd); COMM_SetCommError(handle,CE_IOE); return FALSE; } else @@ -685,7 +681,6 @@ else result = TRUE; } - close(fd); return result; } @@ -726,7 +721,6 @@ tcflush(fd,TCOFLUSH); if(flags&PURGE_RXCLEAR) tcflush(fd,TCIFLUSH); - close(fd); return 1; } @@ -782,8 +776,6 @@ handle, lpStat->cbInQue, lpStat->cbOutQue); } - close(fd); - COMM_GetCommError(handle, errors); COMM_SetCommError(handle, 0); @@ -817,7 +809,6 @@ FIXME("handle %p not found?\n",handle); return FALSE; } - close(fd); return TRUE; } @@ -916,7 +907,6 @@ if ((tcgetattr(fd,&port)) == -1) { int save_error = errno; COMM_SetCommError(handle,CE_IOE); - close( fd ); ERR("tcgetattr error '%s'\n", strerror(save_error)); return FALSE; } @@ -1023,7 +1013,6 @@ COMM_SetCommError(handle,IE_BAUDRATE); - close( fd ); ERR("baudrate %ld\n",lpdcb->BaudRate); return FALSE; } @@ -1089,7 +1078,6 @@ #endif default: COMM_SetCommError(handle,IE_BAUDRATE); - close( fd ); ERR("baudrate %ld\n",lpdcb->BaudRate); return FALSE; } @@ -1132,7 +1120,6 @@ port.c_iflag &= ~INPCK; } else { COMM_SetCommError(handle,IE_BYTESIZE); - close( fd ); ERR("Cannot set MARK Parity\n"); return FALSE; } @@ -1143,7 +1130,6 @@ port.c_iflag &= ~INPCK; } else { COMM_SetCommError(handle,IE_BYTESIZE); - close( fd ); ERR("Cannot set SPACE Parity\n"); return FALSE; } @@ -1151,7 +1137,6 @@ #endif default: COMM_SetCommError(handle,IE_BYTESIZE); - close( fd ); ERR("Parity\n"); return FALSE; } @@ -1173,7 +1158,6 @@ break; default: COMM_SetCommError(handle,IE_BYTESIZE); - close( fd ); ERR("ByteSize\n"); return FALSE; } @@ -1188,7 +1172,6 @@ break; default: COMM_SetCommError(handle,IE_BYTESIZE); - close( fd ); ERR("StopBits\n"); return FALSE; } @@ -1219,12 +1202,10 @@ if (tcsetattr(fd,TCSANOW,&port)==-1) { /* otherwise it hangs with pending input*/ int save_error=errno; COMM_SetCommError(handle,CE_IOE); - close( fd ); ERR("tcsetattr error '%s'\n", strerror(save_error)); return FALSE; } else { COMM_SetCommError(handle,0); - close( fd ); return TRUE; } } @@ -1262,10 +1243,8 @@ int save_error=errno; ERR("tcgetattr error '%s'\n", strerror(save_error)); COMM_SetCommError(handle,CE_IOE); - close( fd ); return FALSE; } - close( fd ); #ifndef __EMX__ #ifdef CBAUD speed= (port.c_cflag & CBAUD); @@ -1465,7 +1444,6 @@ else { r = (1 == write(fd, &chTransmit, 1)); - close(fd); } return r; @@ -1593,7 +1571,6 @@ FIXME("tcsetattr on fd %d failed!\n",fd); return FALSE; } - close(fd); return TRUE; } @@ -1619,7 +1596,6 @@ if(fd<0) return FALSE; result = ioctl(fd, TIOCMGET, &mstat); - close(fd); if (result == -1) { WARN("ioctl failed\n"); @@ -1702,7 +1678,6 @@ ovp = (async_commio*) HeapAlloc(GetProcessHeap(), 0, sizeof (async_commio)); if(!ovp) { - close(fd); return FALSE; } Index: dlls/ntdll/file.c =================================================================== RCS file: /home/wine/wine/dlls/ntdll/file.c,v retrieving revision 1.20 diff -u -r1.20 file.c --- dlls/ntdll/file.c 7 Jan 2003 20:36:28 -0000 1.20 +++ dlls/ntdll/file.c 24 Jan 2003 14:13:35 -0000 @@ -237,8 +237,6 @@ } while ( (result == -1) && ((errno == EAGAIN) || (errno == EINTR)) ); - close( fd ); - if (result == -1) { return IoStatusBlock->u.Status = NTFILE_errno_to_status(errno); Index: dlls/ntdll/virtual.c =================================================================== RCS file: /home/wine/wine/dlls/ntdll/virtual.c,v retrieving revision 1.4 diff -u -r1.4 virtual.c --- dlls/ntdll/virtual.c 7 Jan 2003 20:36:28 -0000 1.4 +++ dlls/ntdll/virtual.c 24 Jan 2003 14:13:39 -0000 @@ -736,13 +736,11 @@ VIRTUAL_SetProt( view, ptr + sec->VirtualAddress, size, vprot ); } - close( fd ); *addr_ptr = ptr; return STATUS_SUCCESS; error: if (ptr != (char *)-1) munmap( ptr, total_size ); - close( fd ); return status; } @@ -1408,7 +1406,6 @@ } res = map_image( handle, unix_handle, base, size_low, header_size, shared_fd, shared_size, removable, addr_ptr ); - if (shared_fd != -1) close( shared_fd ); if (!res) *size_ptr = size_low; return res; } @@ -1484,12 +1481,10 @@ res = STATUS_NO_MEMORY; goto error; } - if (unix_handle != -1) close( unix_handle ); *size_ptr = size; return STATUS_SUCCESS; error: - if (unix_handle != -1) close( unix_handle ); if (ptr != (void *)-1) munmap( ptr, size ); return res; } Index: dlls/winsock/socket.c =================================================================== RCS file: /home/wine/wine/dlls/winsock/socket.c,v retrieving revision 1.117 diff -u -r1.117 socket.c --- dlls/winsock/socket.c 23 Jan 2003 21:20:36 -0000 1.117 +++ dlls/winsock/socket.c 24 Jan 2003 14:13:51 -0000 @@ -336,7 +336,6 @@ if ( ( (access & GENERIC_READ) && (*flags & FD_FLAG_RECV_SHUTDOWN ) ) || ( (access & GENERIC_WRITE) && (*flags & FD_FLAG_SEND_SHUTDOWN ) ) ) { - close (fd); WSASetLastError ( WSAESHUTDOWN ); return -1; } @@ -578,7 +577,6 @@ else wsfds16->fd_array[j++] = wsfds16->fd_array[i]; } - close(fd); lfd[i] = -1; } } @@ -593,24 +591,6 @@ return num_err; } -static void fd_set_unimport( void* wsfds, int lfd[], BOOL b32 ) -{ - if ( wsfds ) - { -#define wsfds16 ((ws_fd_set16*)wsfds) -#define wsfds32 ((WS_fd_set*)wsfds) - int i, count = (b32) ? wsfds32->fd_count : wsfds16->fd_count; - - for( i = 0; i < count; i++ ) - if ( lfd[i] >= 0 ) - close(lfd[i]); - - TRACE("\n"); -#undef wsfds32 -#undef wsfds16 - } -} - static int do_block( int fd, int mask ) { fd_set fds[3]; @@ -1417,7 +1397,6 @@ SetLastError(_get_sock_error(s, FD_ACCEPT_BIT)); /* FIXME: care about the error? */ } - close(fd); SERVER_START_REQ( accept_socket ) { req->lhandle = SOCKET2HANDLE(s); @@ -1516,7 +1495,6 @@ ws_sockaddr_free(uaddr,name); } } - close(fd); } else { @@ -1613,7 +1591,6 @@ { SetLastError(wsaErrno()); } - close(fd); } else { @@ -1622,7 +1599,6 @@ return SOCKET_ERROR; connect_success: - close(fd); _enable_event(SOCKET2HANDLE(s), FD_CONNECT|FD_READ|FD_WRITE, FD_WINE_CONNECTED|FD_READ|FD_WRITE, FD_CONNECT|FD_WINE_LISTENING); @@ -1690,7 +1666,6 @@ res=0; } ws_sockaddr_free(uaddr,name); - close(fd); } else { @@ -1755,7 +1730,6 @@ { res=0; } - close(fd); } else { @@ -1820,12 +1794,10 @@ } else { if (getsockopt(fd, (int) level, optname, optval, optlen) == 0 ) { - close(fd); return 0; } SetLastError((errno == EBADF) ? WSAENOTSOCK : wsaErrno()); } - close(fd); } return SOCKET_ERROR; } @@ -1966,7 +1938,6 @@ if (numInt < 0) { ERR ("Unable to open /proc filesystem to determine number of network interfaces!\n"); - close(fd); WSASetLastError(WSAEINVAL); return (SOCKET_ERROR); } @@ -1976,7 +1947,6 @@ if (!WSAIOCTL_GetInterfaceName(i, ifName)) { ERR ("Error parsing /proc filesystem!\n"); - close(fd); WSASetLastError(WSAEINVAL); return (SOCKET_ERROR); } @@ -1988,7 +1958,6 @@ if (ioctl(fd, SIOCGIFADDR, &ifInfo) < 0) { ERR ("Error obtaining IP address\n"); - close(fd); WSASetLastError(WSAEINVAL); return (SOCKET_ERROR); } @@ -2006,7 +1975,6 @@ if (ioctl(fd, SIOCGIFBRDADDR, &ifInfo) < 0) { ERR ("Error obtaining Broadcast IP address\n"); - close(fd); WSASetLastError(WSAEINVAL); return (SOCKET_ERROR); } @@ -2024,7 +1992,6 @@ if (ioctl(fd, SIOCGIFNETMASK, &ifInfo) < 0) { ERR ("Error obtaining Subnet IP address\n"); - close(fd); WSASetLastError(WSAEINVAL); return (SOCKET_ERROR); } @@ -2059,7 +2026,6 @@ if (ioctl(fd, SIOCGIFFLAGS, &ifInfo) < 0) { ERR ("Error obtaining status flags for socket!\n"); - close(fd); WSASetLastError(WSAEINVAL); return (SOCKET_ERROR); } @@ -2079,14 +2045,12 @@ default: { WARN("\tunsupported WS_IOCTL cmd (%08lx)\n", dwIoControlCode); - close(fd); WSASetLastError(WSAEOPNOTSUPP); return (SOCKET_ERROR); } } /* Function executed with no errors */ - close(fd); return (0); } else @@ -2242,14 +2206,11 @@ { /* AsyncSelect()'ed sockets are always nonblocking */ if (*argp) { - close(fd); return 0; } SetLastError(WSAEINVAL); - close(fd); return SOCKET_ERROR; } - close(fd); if (*argp) _enable_event(SOCKET2HANDLE(s), 0, FD_WINE_NONBLOCKING, 0); else @@ -2279,11 +2240,9 @@ } if( ioctl(fd, newcmd, (char*)argp ) == 0 ) { - close(fd); return 0; } SetLastError((errno == EBADF) ? WSAENOTSOCK : wsaErrno()); - close(fd); } return SOCKET_ERROR; } @@ -2309,7 +2268,6 @@ { if (listen(fd, backlog) == 0) { - close(fd); _enable_event(SOCKET2HANDLE(s), FD_ACCEPT, FD_WINE_LISTENING, FD_CONNECT|FD_WINE_CONNECTED); @@ -2437,7 +2395,6 @@ else wsfds16->fd_array[j++] = wsfds16->fd_array[i]; } - if( fd >= 0 ) close(fd); exceptfd[i] = -1; } if( b32 ) @@ -2449,9 +2406,6 @@ } return highfd; } - fd_set_unimport(ws_readfds, readfd, b32); - fd_set_unimport(ws_writefds, writefd, b32); - fd_set_unimport(ws_exceptfds, exceptfd, b32); if( ws_readfds ) ((WS_fd_set*)ws_readfds)->fd_count = 0; if( ws_writefds ) ((WS_fd_set*)ws_writefds)->fd_count = 0; if( ws_exceptfds ) ((WS_fd_set*)ws_exceptfds)->fd_count = 0; @@ -2553,7 +2507,7 @@ if ( !iovec ) { err = WSAEFAULT; - goto err_close; + goto error; } for ( i = 0; i < dwBufferCount; i++ ) @@ -2617,15 +2571,11 @@ *lpNumberOfBytesSent = n; HeapFree ( GetProcessHeap(), 0, iovec ); - close ( fd ); return 0; err_free: HeapFree ( GetProcessHeap(), 0, iovec ); -err_close: - close ( fd ); - error: WARN (" -> ERROR %d\n", err); WSASetLastError (err); @@ -2725,7 +2675,6 @@ }else{ if (!convert_sockopt(&level, &optname)) { SetLastError(WSAENOPROTOOPT); - close(fd); return SOCKET_ERROR; } if (optname == SO_LINGER && optval) { @@ -2755,7 +2704,6 @@ WARN("SO_RCVTIMEO for %d bytes: assuming unixism\n", optlen); } else { WARN("SO_RCVTIMEO for %d bytes is weird: ignored\n", optlen); - close(fd); return 0; } } @@ -2774,7 +2722,6 @@ WARN("SO_SNDTIMEO for %d bytes: assuming unixism\n", optlen); } else { WARN("SO_SNDTIMEO for %d bytes is weird: ignored\n", optlen); - close(fd); return 0; } } @@ -2782,17 +2729,14 @@ } if(optname == SO_RCVBUF && *(int*)optval < 2048) { WARN("SO_RCVBF for %d bytes is too small: ignored\n", *(int*)optval ); - close( fd); return 0; } if (setsockopt(fd, level, optname, optval, optlen) == 0) { - close(fd); return 0; } SetLastError(wsaErrno()); - close(fd); } else SetLastError(WSAENOTSOCK); return SOCKET_ERROR; @@ -2859,7 +2803,6 @@ err = WS2_register_async_shutdown ( s, fd0, ASYNC_TYPE_READ ); if ( err ) { - close ( fd0 ); goto error; } } @@ -2868,7 +2811,6 @@ err = WS2_register_async_shutdown ( s, fd1, ASYNC_TYPE_WRITE ); if ( err ) { - close ( fd1 ); goto error; } } @@ -2878,10 +2820,8 @@ if ( shutdown( fd, how ) ) { err = wsaErrno (); - close ( fd ); goto error; } - close(fd); } _enable_event( SOCKET2HANDLE(s), 0, 0, clear_flags ); @@ -4006,7 +3946,7 @@ if ( !iovec ) { err = WSAEFAULT; - goto err_close; + goto error; } for (i = 0; i < dwBufferCount; i++) @@ -4070,16 +4010,12 @@ *lpNumberOfBytesRecvd = n; HeapFree (GetProcessHeap(), 0, iovec); - close(fd); _enable_event(SOCKET2HANDLE(s), FD_READ, 0, 0); return 0; err_free: HeapFree (GetProcessHeap(), 0, iovec); - -err_close: - close (fd); error: WARN(" -> ERROR %d\n", err); Index: files/file.c =================================================================== RCS file: /home/wine/wine/files/file.c,v retrieving revision 1.177 diff -u -r1.177 file.c --- files/file.c 11 Jan 2003 21:03:18 -0000 1.177 +++ files/file.c 24 Jan 2003 14:13:59 -0000 @@ -333,7 +333,6 @@ else if (((access & GENERIC_READ) && (flags & FD_FLAG_RECV_SHUTDOWN)) || ((access & GENERIC_WRITE) && (flags & FD_FLAG_SEND_SHUTDOWN))) { - close (fd); SetLastError ( ERROR_PIPE_NOT_CONNECTED ); return -1; } @@ -1755,7 +1754,6 @@ return !register_new_async (&ovp->async); error: - close (fd); return FALSE; } @@ -1813,12 +1811,10 @@ if ( (overlapped==NULL) || NtResetEvent( overlapped->hEvent, NULL ) ) { TRACE("Overlapped not specified or invalid event flag\n"); - close(unix_handle); SetLastError(ERROR_INVALID_PARAMETER); return FALSE; } - close(unix_handle); overlapped->InternalHigh = 0; if(!FILE_ReadFileEx(hFile, buffer, bytesToRead, overlapped, NULL, overlapped->hEvent)) @@ -1835,7 +1831,6 @@ } if (flags & FD_FLAG_TIMEOUT) { - close(unix_handle); return FILE_TimeoutRead(hFile, buffer, bytesToRead, bytesRead); } switch(type) @@ -1856,7 +1851,6 @@ &highOffset, FILE_BEGIN)) && (GetLastError() != NO_ERROR) ) { - close(unix_handle); return FALSE; } } @@ -1872,7 +1866,6 @@ off_t offset = OVERLAPPED_OFFSET(overlapped); if(lseek(unix_handle, offset, SEEK_SET) == -1) { - close(unix_handle); SetLastError(ERROR_INVALID_PARAMETER); return FALSE; } @@ -1886,7 +1879,6 @@ FILE_SetDosError(); break; } - close( unix_handle ); if (result == -1) return FALSE; if (bytesRead) *bytesRead = result; return TRUE; @@ -1998,7 +1990,6 @@ return !register_new_async (&ovp->async); error: - close (fd); return FALSE; } @@ -2037,12 +2028,10 @@ if ( (overlapped==NULL) || NtResetEvent( overlapped->hEvent, NULL ) ) { TRACE("Overlapped not specified or invalid event flag\n"); - close(unix_handle); SetLastError(ERROR_INVALID_PARAMETER); return FALSE; } - close(unix_handle); overlapped->InternalHigh = 0; if(!FILE_WriteFileEx(hFile, buffer, bytesToWrite, overlapped, NULL, overlapped->hEvent)) @@ -2075,7 +2064,6 @@ &highOffset, FILE_BEGIN)) && (GetLastError() != NO_ERROR) ) { - close(unix_handle); return FALSE; } } @@ -2086,7 +2074,6 @@ return FALSE; if (overlapped) { - close(unix_handle); SetLastError(ERROR_INVALID_PARAMETER); return FALSE; } @@ -2098,7 +2085,6 @@ off_t offset = OVERLAPPED_OFFSET(overlapped); if(lseek(unix_handle, offset, SEEK_SET) == -1) { - close(unix_handle); SetLastError(ERROR_INVALID_PARAMETER); return FALSE; } @@ -2115,7 +2101,6 @@ FILE_SetDosError(); break; } - close( unix_handle ); if (result == -1) return FALSE; if (bytesWritten) *bytesWritten = result; return TRUE; Index: files/smb.c =================================================================== RCS file: /home/wine/wine/files/smb.c,v retrieving revision 1.16 diff -u -r1.16 smb.c --- files/smb.c 24 Jan 2003 00:54:58 -0000 1.16 +++ files/smb.c 24 Jan 2003 14:14:02 -0000 @@ -1532,7 +1532,6 @@ if(total>=bytesToRead) break; } - close(fd); if(bytesRead) *bytesRead = total; Index: include/async.h =================================================================== RCS file: /home/wine/wine/include/async.h,v retrieving revision 1.5 diff -u -r1.5 async.h --- include/async.h 4 May 2002 18:37:08 -0000 1.5 +++ include/async.h 24 Jan 2003 14:14:02 -0000 @@ -72,7 +72,6 @@ ovp->next = ovp->prev = NULL; - close( ovp->fd ); if( ovp->event != INVALID_HANDLE_VALUE ) NtSetEvent( ovp->event, NULL ); Index: scheduler/client.c =================================================================== RCS file: /home/wine/wine/scheduler/client.c,v retrieving revision 1.92 diff -u -r1.92 client.c --- scheduler/client.c 31 Oct 2002 03:41:56 -0000 1.92 +++ scheduler/client.c 24 Jan 2003 14:14:05 -0000 @@ -436,7 +436,6 @@ */ } - if ((fd != -1) && ((fd = dup(fd)) == -1)) return STATUS_TOO_MANY_OPENED_FILES; *unix_fd = fd; return STATUS_SUCCESS; } -- Martin Wilck Phone: +49 5251 8 15113 Fujitsu Siemens Computers Fax: +49 5251 8 20409 Heinz-Nixdorf-Ring 1 mailto:[email protected] D-33106 Paderborn http://www.fujitsu-siemens.com/primergy