On Thu,  4 Jun 2015 11:37:52 -0500
Derek Foreman <[email protected]> wrote:

> Signed-off-by: Derek Foreman <[email protected]>
> ---
> 
> 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 <[email protected]>


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

Reply via email to