[PATCH v2 weston] log: Open log file CLOEXEC so child processes don't get the fd

[Pekka Paalanen]

On Thu,  4 Jun 2015 11:37:52 -0500
Derek Foreman <derekf at osg.samsung.com> wrote:

> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
> 
> Refactor and re-use the code from set_cloexec_or_close() instead of
> using fopen "e"
> 
>  shared/os-compatibility.c | 22 ++++++++++++++--------
>  shared/os-compatibility.h |  3 +++
>  src/log.c                 |  5 +++++
>  3 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/shared/os-compatibility.c b/shared/os-compatibility.c
> index 611e7c8..d0ff1c8 100644
> --- a/shared/os-compatibility.c
> +++ b/shared/os-compatibility.c
> @@ -33,8 +33,8 @@
>  
>  #include "os-compatibility.h"
>  
> -static int
> -set_cloexec_or_close(int fd)
> +int
> +os_fd_set_cloexec(int fd)
>  {
>  	long flags;
>  
> @@ -43,16 +43,22 @@ set_cloexec_or_close(int fd)
>  
>  	flags = fcntl(fd, F_GETFD);
>  	if (flags == -1)
> -		goto err;
> +		return -1;
>  
>  	if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1)
> -		goto err;
> +		return -1;
>  
> -	return fd;
> +	return 0;
> +}
>  
> -err:
> -	close(fd);
> -	return -1;
> +static int
> +set_cloexec_or_close(int fd)
> +{
> +	if (os_fd_set_cloexec(fd) != 0) {
> +		close(fd);
> +		return -1;
> +	}
> +	return fd;
>  }
>  
>  int
> diff --git a/shared/os-compatibility.h b/shared/os-compatibility.h
> index 172bb7e..60ae7fd 100644
> --- a/shared/os-compatibility.h
> +++ b/shared/os-compatibility.h
> @@ -38,6 +38,9 @@ backtrace(void **buffer, int size)
>  #endif
>  
>  int
> +os_fd_set_cloexec(int fd);
> +
> +int
>  os_socketpair_cloexec(int domain, int type, int protocol, int *sv);
>  
>  int
> diff --git a/src/log.c b/src/log.c
> index 99bbe18..317f945 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -33,6 +33,8 @@
>  
>  #include "compositor.h"
>  
> +#include "os-compatibility.h"
> +
>  static FILE *weston_logfile = NULL;
>  
>  static int cached_tm_mday = -1;
> @@ -77,6 +79,9 @@ weston_log_file_open(const char *filename)
>  	if (filename != NULL)
>  		weston_logfile = fopen(filename, "a");
>  
> +	if (weston_logfile)
> +		os_fd_set_cloexec(fileno(weston_logfile));
> +

Would be better to have this in the same branch as the fopen(), even
though we do not intend to open and close log files multiple times.
Closing the log file makes weston_logfile be stderr again.

>  	if (weston_logfile == NULL)
>  		weston_logfile = stderr;
>  	else

I don't see a reasonable way to handle os_fd_set_cloexec() failing, so
with the above fixed:
Reviewed-By: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq