From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41755) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StUjD-0000Fe-Hx for qemu-devel@nongnu.org; Mon, 23 Jul 2012 22:20:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1StUj9-0003kv-Q1 for qemu-devel@nongnu.org; Mon, 23 Jul 2012 22:20:02 -0400 Received: from e1.ny.us.ibm.com ([32.97.182.141]:51088) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StUj9-0003kH-Ll for qemu-devel@nongnu.org; Mon, 23 Jul 2012 22:19:59 -0400 Received: from /spool/local by e1.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 23 Jul 2012 22:19:56 -0400 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 31EF2C9001A for ; Mon, 23 Jul 2012 22:19:54 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6O2Jr4P365392 for ; Mon, 23 Jul 2012 22:19:53 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6O2Jr8S006240 for ; Mon, 23 Jul 2012 23:19:53 -0300 Message-ID: <500E0648.5060707@linux.vnet.ibm.com> Date: Mon, 23 Jul 2012 22:19:52 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1343048885-1701-1-git-send-email-coreyb@linux.vnet.ibm.com> <1343048885-1701-2-git-send-email-coreyb@linux.vnet.ibm.com> <500DD522.3060303@redhat.com> In-Reply-To: <500DD522.3060303@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com On 07/23/2012 06:50 PM, Eric Blake wrote: > On 07/23/2012 07:08 AM, Corey Bryant wrote: >> Set the close-on-exec flag for the file descriptor received >> via SCM_RIGHTS. >> > >> +++ b/qemu-char.c >> @@ -2263,9 +2263,17 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) >> msg.msg_control = &msg_control; >> msg.msg_controllen = sizeof(msg_control); >> >> +#ifdef MSG_CMSG_CLOEXEC >> + ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC); >> +#else >> ret = recvmsg(s->fd, &msg, 0); >> - if (ret > 0 && s->is_unix) >> + if (ret > 0) { >> + qemu_set_cloexec(s->fd); > > Wrong fd. You aren't changing cloexec on the socket (s->fd), but on the > fd that was received via msg (which you don't know at this point in time). > Ugh, that's bad. >> + } >> +#endif >> + if (ret > 0 && s->is_unix) { >> unix_process_msgfd(chr, &msg); > > Only here do you know what fd you received. > > I would write it more like: > > int flags = 0; > #ifdef MSG_CMSG_CLOEXEC > flags |= MSG_CMSG_CLOEXEC > #endif > ret = recvmsg(s->fd, &msg, flags); > if (ret > 0 && s->is_unix) { > unix_process_msgfd(chr, &msg); > #ifndef MSG_CMSG_CLOEXEC > qemu_set_cloexec(/* fd determined from msg */) > #endif > } > > which almost implies that unix_process_msgfd() should be the function > that sets cloexec, but without wasting the time doing so if recvmsg > already did the job. > Thanks for the suggestion and catching this. I'll take this into account in the next version. -- Regards, Corey