[PATCH wayland v3 3/7] os: Expose set_cloexec_or_close with a namespaced name

[Pekka Paalanen]

On Mon,  7 Dec 2015 22:49:15 -0800
Bryce Harrington <bryce at osg.samsung.com> wrote:

> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  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 <pekka.paalanen at collabora.co.uk>

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151214/c3787091/attachment-0001.sig>