Patchwork [v4,1/7] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg

login
register
mail settings
Submitter Corey Bryant
Date June 22, 2012, 6:36 p.m.
Message ID <1340390174-7493-2-git-send-email-coreyb@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/166655/
State New
Headers show

Comments

Corey Bryant - June 22, 2012, 6:36 p.m.
This sets the close-on-exec flag for the file descriptor received
via SCM_RIGHTS.

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
v4
 -This patch is new in v4 (eblake@redhat.com)

 qemu-char.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Eric Blake - June 22, 2012, 7:31 p.m.
On 06/22/2012 12:36 PM, Corey Bryant wrote:
> This sets the close-on-exec flag for the file descriptor received
> via SCM_RIGHTS.
> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> v4
>  -This patch is new in v4 (eblake@redhat.com)
> 
>  qemu-char.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index c2aaaee..f890113 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2263,7 +2263,7 @@ 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, 0);
> +    ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC);

MSG_CMSG_CLOEXEC is not (yet) in POSIX (although it has been proposed
for addition); therefore, at the moment, it only exists on Linux and
Cygwin.  Does this need to have conditional code to allow compilation on
BSD, such as:

#ifndef MSG_CMSG_CLOEXEC
# define MSG_CMSG_CLOEXEC 0
#endif

as well as fallback code that sets FD_CLOEXEC manually via fcntl() when
MSG_CMSG_CLOEXEC is missing?
Stefan Hajnoczi - June 25, 2012, 9:16 a.m.
On Fri, Jun 22, 2012 at 01:31:17PM -0600, Eric Blake wrote:
> On 06/22/2012 12:36 PM, Corey Bryant wrote:
> > This sets the close-on-exec flag for the file descriptor received
> > via SCM_RIGHTS.
> > 
> > Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> > ---
> > v4
> >  -This patch is new in v4 (eblake@redhat.com)
> > 
> >  qemu-char.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/qemu-char.c b/qemu-char.c
> > index c2aaaee..f890113 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -2263,7 +2263,7 @@ 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, 0);
> > +    ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC);
> 
> MSG_CMSG_CLOEXEC is not (yet) in POSIX (although it has been proposed
> for addition); therefore, at the moment, it only exists on Linux and
> Cygwin.  Does this need to have conditional code to allow compilation on
> BSD, such as:
> 
> #ifndef MSG_CMSG_CLOEXEC
> # define MSG_CMSG_CLOEXEC 0
> #endif
> 
> as well as fallback code that sets FD_CLOEXEC manually via fcntl() when
> MSG_CMSG_CLOEXEC is missing?

Good point.  I think the answer is yes.  Just like qemu_open() we can
wrap "recvmsg(2) with fd" so that platforms with MSG_CMSG_CLOEXEC use
that flag and other platforms use qemu_set_cloexec().

Stefan
Corey Bryant - June 25, 2012, 1:44 p.m.
On 06/25/2012 05:16 AM, Stefan Hajnoczi wrote:
> On Fri, Jun 22, 2012 at 01:31:17PM -0600, Eric Blake wrote:
>> On 06/22/2012 12:36 PM, Corey Bryant wrote:
>>> This sets the close-on-exec flag for the file descriptor received
>>> via SCM_RIGHTS.
>>>
>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>> ---
>>> v4
>>>   -This patch is new in v4 (eblake@redhat.com)
>>>
>>>   qemu-char.c |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/qemu-char.c b/qemu-char.c
>>> index c2aaaee..f890113 100644
>>> --- a/qemu-char.c
>>> +++ b/qemu-char.c
>>> @@ -2263,7 +2263,7 @@ 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, 0);
>>> +    ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC);
>>
>> MSG_CMSG_CLOEXEC is not (yet) in POSIX (although it has been proposed
>> for addition); therefore, at the moment, it only exists on Linux and
>> Cygwin.  Does this need to have conditional code to allow compilation on
>> BSD, such as:
>>
>> #ifndef MSG_CMSG_CLOEXEC
>> # define MSG_CMSG_CLOEXEC 0
>> #endif
>>
>> as well as fallback code that sets FD_CLOEXEC manually via fcntl() when
>> MSG_CMSG_CLOEXEC is missing?
>
> Good point.  I think the answer is yes.  Just like qemu_open() we can
> wrap "recvmsg(2) with fd" so that platforms with MSG_CMSG_CLOEXEC use
> that flag and other platforms use qemu_set_cloexec().
>
> Stefan
>

Thanks for pointing this out.  I'll make the updates in v5.

Patch

diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..f890113 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2263,7 +2263,7 @@  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, 0);
+    ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC);
     if (ret > 0 && s->is_unix)
         unix_process_msgfd(chr, &msg);