From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45592) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adiPf-0002x2-Hl for qemu-devel@nongnu.org; Wed, 09 Mar 2016 13:00:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adiPc-0007J1-8Q for qemu-devel@nongnu.org; Wed, 09 Mar 2016 13:00:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35593) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adiPc-0007Iw-2b for qemu-devel@nongnu.org; Wed, 09 Mar 2016 13:00:44 -0500 References: <1457544504-8548-1-git-send-email-berrange@redhat.com> <1457544504-8548-21-git-send-email-berrange@redhat.com> From: Paolo Bonzini Message-ID: <56E064C8.1050004@redhat.com> Date: Wed, 9 Mar 2016 19:00:40 +0100 MIME-Version: 1.0 In-Reply-To: <1457544504-8548-21-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 20/21] osdep: remove direct use of qemu_socket & qemu_accept List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Stefan Weil , Andrew Baumann On 09/03/2016 18:28, Daniel P. Berrange wrote: > The qemu_socket & qemu_accept wrappers exist to ensure > the O_CLOEXEC flag is always set on new sockets. All > code is supposed to use these methods instead of the > normal POSIX socket/accept methods. This proves rather > error prone though with a number of places forgetting > to call the QEMU specific wrappers. > > QEMU is already using a technique to transparently > wrap sockets APIs on Win32, avoiding the need for callers > to remember to use QEMU specific wrappers. This switches > over to that technique for socket/accept on POSIX platforms, > removing the need for callers to use the qemu_socket and > qemu_accept methods directly. > > Signed-off-by: Daniel P. Berrange > --- > contrib/ivshmem-server/ivshmem-server.c | 4 ++-- > include/qemu/sockets.h | 2 -- > include/sysemu/os-posix.h | 11 +++++++++ > migration/tcp.c | 2 +- > migration/unix.c | 2 +- > net/socket.c | 10 ++++---- > qga/channel-posix.c | 4 ++-- > slirp/ip_icmp.c | 2 +- > slirp/misc.c | 4 ++-- > slirp/socket.c | 2 +- > slirp/tcp_subr.c | 2 +- > slirp/udp.c | 4 ++-- > util/osdep.c | 42 --------------------------------- > util/oslib-posix.c | 39 ++++++++++++++++++++++++++++++ > util/oslib-win32.c | 4 ++++ > util/qemu-sockets.c | 10 ++++---- > 16 files changed, 77 insertions(+), 67 deletions(-) > > diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c > index bfd0fad..1b72ca0 100644 > --- a/contrib/ivshmem-server/ivshmem-server.c > +++ b/contrib/ivshmem-server/ivshmem-server.c > @@ -142,8 +142,8 @@ ivshmem_server_handle_new_conn(IvshmemServer *server) > > /* accept the incoming connection */ > unaddr_len = sizeof(unaddr); > - newfd = qemu_accept(server->sock_fd, > - (struct sockaddr *)&unaddr, &unaddr_len); > + newfd = accept(server->sock_fd, > + (struct sockaddr *)&unaddr, &unaddr_len); > > if (newfd < 0) { > IVSHMEM_SERVER_DEBUG(server, "cannot accept() %s\n", strerror(errno)); I disagree with this change, it adds too much magic. Someone may see a fcntl(fd, F_SETFD, 0); after a socket() call and think it's unnecessary. Paolo