[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

+++ 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);

+    ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC);
      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.

+    }
+    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;
ret = recvmsg(s->fd, &msg, flags);
if (ret > 0 && s->is_unix) {
     unix_process_msgfd(chr, &msg);
     qemu_set_cloexec(/* fd determined from msg */)

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.


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