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

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).

> + } > +#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.