On Mon,  7 Dec 2015 22:49:15 -0800
Bryce Harrington <[email protected]> wrote:

> Signed-off-by: Bryce Harrington <[email protected]>
> ---
>  src/wayland-os.c | 24 +++++++++++++++++-------
>  src/wayland-os.h |  3 +++
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/src/wayland-os.c b/src/wayland-os.c
> index 93b6f5f..342a73a 100644
> --- a/src/wayland-os.c
> +++ b/src/wayland-os.c
> @@ -35,8 +35,18 @@
>  #include "../config.h"
>  #include "wayland-os.h"
>  
> -static int
> -set_cloexec_or_close(int fd)
> +
> +/** Ensure the file descriptor will close after an execve
> + *
> + * If the fd cannot be set as cloexec, close it and return
> + * an error.

Returns either the fd passed in, or -1 on error.

NOTE: Setting cloexec after the file descriptor already exists is
inherently racy against forks with threads, and should only be used as
a fallback when the OS does not offer a race-free way.


With those added and *if* this function is actually needed:
Reviewed-by: Pekka Paalanen <[email protected]>

I say "if", because I think it might be better that
wl_display_add_socket_fd() simply required its argument fd to be
cloexec already. But that's to discuss in the relevant patch, not here.

Btw. committing this documentation even if wl_os_set_cloexec_or_close()
wasn't needed would be nice.


Thanks,
pq

> + *
> + * \param fd The file descriptor to be modified
> + *
> + * \memberof wl_os
> + */
> +int
> +wl_os_set_cloexec_or_close(int fd)
>  {
>       long flags;
>  
> @@ -69,7 +79,7 @@ wl_os_socket_cloexec(int domain, int type, int protocol)
>               return -1;
>  
>       fd = socket(domain, type, protocol);
> -     return set_cloexec_or_close(fd);
> +     return wl_os_set_cloexec_or_close(fd);
>  }
>  
>  int
> @@ -84,7 +94,7 @@ wl_os_dupfd_cloexec(int fd, long minfd)
>               return -1;
>  
>       newfd = fcntl(fd, F_DUPFD, minfd);
> -     return set_cloexec_or_close(newfd);
> +     return wl_os_set_cloexec_or_close(newfd);
>  }
>  
>  static ssize_t
> @@ -112,7 +122,7 @@ recvmsg_cloexec_fallback(int sockfd, struct msghdr *msg, 
> int flags)
>               data = CMSG_DATA(cmsg);
>               end = (int *)(data + cmsg->cmsg_len - CMSG_LEN(0));
>               for (fd = (int *)data; fd < end; ++fd)
> -                     *fd = set_cloexec_or_close(*fd);
> +                     *fd = wl_os_set_cloexec_or_close(*fd);
>       }
>  
>       return len;
> @@ -146,7 +156,7 @@ wl_os_epoll_create_cloexec(void)
>  #endif
>  
>       fd = epoll_create(1);
> -     return set_cloexec_or_close(fd);
> +     return wl_os_set_cloexec_or_close(fd);
>  }
>  
>  int
> @@ -163,5 +173,5 @@ wl_os_accept_cloexec(int sockfd, struct sockaddr *addr, 
> socklen_t *addrlen)
>  #endif
>  
>       fd = accept(sockfd, addr, addrlen);
> -     return set_cloexec_or_close(fd);
> +     return wl_os_set_cloexec_or_close(fd);
>  }
> diff --git a/src/wayland-os.h b/src/wayland-os.h
> index f51efaa..e756841 100644
> --- a/src/wayland-os.h
> +++ b/src/wayland-os.h
> @@ -27,6 +27,9 @@
>  #define WAYLAND_OS_H
>  
>  int
> +wl_os_set_cloexec_or_close(int fd);
> +
> +int
>  wl_os_socket_cloexec(int domain, int type, int protocol);
>  
>  int

Attachment: pgpPwQlrvxg_g.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to