[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg





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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]