[PATCH weston 3/6] Move the functions launching clients to main.c

Benoit Gschwind gschwind at gnu-log.net
Fri May 27 12:28:40 UTC 2016


Hello Giulio,

With this patch, and mine to fix MODULEDIR of patch #2, this patch does
not build due to incomplete type:

src/screenshooter.c:46:24: error: field 'process' has incomplete type
  struct weston_process process;

The patch look fine, if I ignore this building error, I also saw a lot
of change about screenshoter in the following patch, that I can imagine
resolve the issue. I do not know the policy of weston, but I guess you
must fix this issue.

Best regards.

On 24/05/2016 18:59, Giulio Camuffo wrote:
> They belong in the compositor rather than libweston since they
> set signals handlers, and a library should not do that behind its
> user's back. Besides, they were using functions in main.c already
> so they were not usable by other compositors.
> 
> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
>  ivi-shell/hmi-controller.c     |   1 +
>  src/compositor.c               | 144 ----------------------------------------
>  src/compositor.h               |  22 -------
>  src/main.c                     | 145 +++++++++++++++++++++++++++++++++++++++++
>  src/text-backend.c             |   1 +
>  src/weston.h                   |  24 +++++++
>  tests/ivi_layout-test-plugin.c |   1 +
>  tests/weston-test.c            |   1 +
>  xwayland/xwayland.h            |   1 +
>  9 files changed, 174 insertions(+), 166 deletions(-)
> 
> diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
> index 97f78af..094682c 100644
> --- a/ivi-shell/hmi-controller.c
> +++ b/ivi-shell/hmi-controller.c
> @@ -62,6 +62,7 @@
>  #include "ivi-hmi-controller-server-protocol.h"
>  #include "shared/helpers.h"
>  #include "shared/xalloc.h"
> +#include "src/weston.h"
>  
>  /*****************************************************************************
>   *  structure, globals
> diff --git a/src/compositor.c b/src/compositor.c
> index 5a52d86..3904ef0 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -236,150 +236,6 @@ weston_output_mode_switch_to_temporary(struct weston_output *output,
>  }
>  
>  static void
> -child_client_exec(int sockfd, const char *path)
> -{
> -	int clientfd;
> -	char s[32];
> -	sigset_t allsigs;
> -
> -	/* do not give our signal mask to the new process */
> -	sigfillset(&allsigs);
> -	sigprocmask(SIG_UNBLOCK, &allsigs, NULL);
> -
> -	/* Launch clients as the user. Do not lauch clients with wrong euid.*/
> -	if (seteuid(getuid()) == -1) {
> -		weston_log("compositor: failed seteuid\n");
> -		return;
> -	}
> -
> -	/* SOCK_CLOEXEC closes both ends, so we dup the fd to get a
> -	 * non-CLOEXEC fd to pass through exec. */
> -	clientfd = dup(sockfd);
> -	if (clientfd == -1) {
> -		weston_log("compositor: dup failed: %m\n");
> -		return;
> -	}
> -
> -	snprintf(s, sizeof s, "%d", clientfd);
> -	setenv("WAYLAND_SOCKET", s, 1);
> -
> -	if (execl(path, path, NULL) < 0)
> -		weston_log("compositor: executing '%s' failed: %m\n",
> -			path);
> -}
> -
> -WL_EXPORT struct wl_client *
> -weston_client_launch(struct weston_compositor *compositor,
> -		     struct weston_process *proc,
> -		     const char *path,
> -		     weston_process_cleanup_func_t cleanup)
> -{
> -	int sv[2];
> -	pid_t pid;
> -	struct wl_client *client;
> -
> -	weston_log("launching '%s'\n", path);
> -
> -	if (os_socketpair_cloexec(AF_UNIX, SOCK_STREAM, 0, sv) < 0) {
> -		weston_log("weston_client_launch: "
> -			"socketpair failed while launching '%s': %m\n",
> -			path);
> -		return NULL;
> -	}
> -
> -	pid = fork();
> -	if (pid == -1) {
> -		close(sv[0]);
> -		close(sv[1]);
> -		weston_log("weston_client_launch: "
> -			"fork failed while launching '%s': %m\n", path);
> -		return NULL;
> -	}
> -
> -	if (pid == 0) {
> -		child_client_exec(sv[1], path);
> -		_exit(-1);
> -	}
> -
> -	close(sv[1]);
> -
> -	client = wl_client_create(compositor->wl_display, sv[0]);
> -	if (!client) {
> -		close(sv[0]);
> -		weston_log("weston_client_launch: "
> -			"wl_client_create failed while launching '%s'.\n",
> -			path);
> -		return NULL;
> -	}
> -
> -	proc->pid = pid;
> -	proc->cleanup = cleanup;
> -	weston_watch_process(proc);
> -
> -	return client;
> -}
> -
> -struct process_info {
> -	struct weston_process proc;
> -	char *path;
> -};
> -
> -static void
> -process_handle_sigchld(struct weston_process *process, int status)
> -{
> -	struct process_info *pinfo =
> -		container_of(process, struct process_info, proc);
> -
> -	/*
> -	 * There are no guarantees whether this runs before or after
> -	 * the wl_client destructor.
> -	 */
> -
> -	if (WIFEXITED(status)) {
> -		weston_log("%s exited with status %d\n", pinfo->path,
> -			   WEXITSTATUS(status));
> -	} else if (WIFSIGNALED(status)) {
> -		weston_log("%s died on signal %d\n", pinfo->path,
> -			   WTERMSIG(status));
> -	} else {
> -		weston_log("%s disappeared\n", pinfo->path);
> -	}
> -
> -	free(pinfo->path);
> -	free(pinfo);
> -}
> -
> -WL_EXPORT struct wl_client *
> -weston_client_start(struct weston_compositor *compositor, const char *path)
> -{
> -	struct process_info *pinfo;
> -	struct wl_client *client;
> -
> -	pinfo = zalloc(sizeof *pinfo);
> -	if (!pinfo)
> -		return NULL;
> -
> -	pinfo->path = strdup(path);
> -	if (!pinfo->path)
> -		goto out_free;
> -
> -	client = weston_client_launch(compositor, &pinfo->proc, path,
> -				      process_handle_sigchld);
> -	if (!client)
> -		goto out_str;
> -
> -	return client;
> -
> -out_str:
> -	free(pinfo->path);
> -
> -out_free:
> -	free(pinfo);
> -
> -	return NULL;
> -}
> -
> -static void
>  region_init_infinite(pixman_region32_t *region)
>  {
>  	pixman_region32_init_rect(region, INT32_MIN, INT32_MIN,
> diff --git a/src/compositor.h b/src/compositor.h
> index 0bbf458..125734c 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -1603,28 +1603,6 @@ text_backend_init(struct weston_compositor *ec);
>  void
>  text_backend_destroy(struct text_backend *text_backend);
>  
> -struct weston_process;
> -typedef void (*weston_process_cleanup_func_t)(struct weston_process *process,
> -					    int status);
> -
> -struct weston_process {
> -	pid_t pid;
> -	weston_process_cleanup_func_t cleanup;
> -	struct wl_list link;
> -};
> -
> -struct wl_client *
> -weston_client_launch(struct weston_compositor *compositor,
> -		     struct weston_process *proc,
> -		     const char *path,
> -		     weston_process_cleanup_func_t cleanup);
> -
> -struct wl_client *
> -weston_client_start(struct weston_compositor *compositor, const char *path);
> -
> -void
> -weston_watch_process(struct weston_process *process);
> -
>  struct weston_view_animation;
>  typedef	void (*weston_view_animation_done_func_t)(struct weston_view_animation *animation, void *data);
>  
> diff --git a/src/main.c b/src/main.c
> index 7b52910..34789b1 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -37,6 +37,7 @@
>  #include <sys/utsname.h>
>  #include <sys/stat.h>
>  #include <sys/wait.h>
> +#include <sys/socket.h>
>  #include <linux/limits.h>
>  
>  #ifdef HAVE_LIBUNWIND
> @@ -170,12 +171,156 @@ print_backtrace(void)
>  
>  #endif
>  
> +static void
> +child_client_exec(int sockfd, const char *path)
> +{
> +	int clientfd;
> +	char s[32];
> +	sigset_t allsigs;
> +
> +	/* do not give our signal mask to the new process */
> +	sigfillset(&allsigs);
> +	sigprocmask(SIG_UNBLOCK, &allsigs, NULL);
> +
> +	/* Launch clients as the user. Do not lauch clients with wrong euid.*/
> +	if (seteuid(getuid()) == -1) {
> +		weston_log("compositor: failed seteuid\n");
> +		return;
> +	}
> +
> +	/* SOCK_CLOEXEC closes both ends, so we dup the fd to get a
> +	 * non-CLOEXEC fd to pass through exec. */
> +	clientfd = dup(sockfd);
> +	if (clientfd == -1) {
> +		weston_log("compositor: dup failed: %m\n");
> +		return;
> +	}
> +
> +	snprintf(s, sizeof s, "%d", clientfd);
> +	setenv("WAYLAND_SOCKET", s, 1);
> +
> +	if (execl(path, path, NULL) < 0)
> +		weston_log("compositor: executing '%s' failed: %m\n",
> +			path);
> +}
> +
> +WL_EXPORT struct wl_client *
> +weston_client_launch(struct weston_compositor *compositor,
> +		     struct weston_process *proc,
> +		     const char *path,
> +		     weston_process_cleanup_func_t cleanup)
> +{
> +	int sv[2];
> +	pid_t pid;
> +	struct wl_client *client;
> +
> +	weston_log("launching '%s'\n", path);
> +
> +	if (os_socketpair_cloexec(AF_UNIX, SOCK_STREAM, 0, sv) < 0) {
> +		weston_log("weston_client_launch: "
> +			"socketpair failed while launching '%s': %m\n",
> +			path);
> +		return NULL;
> +	}
> +
> +	pid = fork();
> +	if (pid == -1) {
> +		close(sv[0]);
> +		close(sv[1]);
> +		weston_log("weston_client_launch: "
> +			"fork failed while launching '%s': %m\n", path);
> +		return NULL;
> +	}
> +
> +	if (pid == 0) {
> +		child_client_exec(sv[1], path);
> +		_exit(-1);
> +	}
> +
> +	close(sv[1]);
> +
> +	client = wl_client_create(compositor->wl_display, sv[0]);
> +	if (!client) {
> +		close(sv[0]);
> +		weston_log("weston_client_launch: "
> +			"wl_client_create failed while launching '%s'.\n",
> +			path);
> +		return NULL;
> +	}
> +
> +	proc->pid = pid;
> +	proc->cleanup = cleanup;
> +	weston_watch_process(proc);
> +
> +	return client;
> +}
> +
>  WL_EXPORT void
>  weston_watch_process(struct weston_process *process)
>  {
>  	wl_list_insert(&child_process_list, &process->link);
>  }
>  
> +struct process_info {
> +	struct weston_process proc;
> +	char *path;
> +};
> +
> +static void
> +process_handle_sigchld(struct weston_process *process, int status)
> +{
> +	struct process_info *pinfo =
> +		container_of(process, struct process_info, proc);
> +
> +	/*
> +	 * There are no guarantees whether this runs before or after
> +	 * the wl_client destructor.
> +	 */
> +
> +	if (WIFEXITED(status)) {
> +		weston_log("%s exited with status %d\n", pinfo->path,
> +			   WEXITSTATUS(status));
> +	} else if (WIFSIGNALED(status)) {
> +		weston_log("%s died on signal %d\n", pinfo->path,
> +			   WTERMSIG(status));
> +	} else {
> +		weston_log("%s disappeared\n", pinfo->path);
> +	}
> +
> +	free(pinfo->path);
> +	free(pinfo);
> +}
> +
> +WL_EXPORT struct wl_client *
> +weston_client_start(struct weston_compositor *compositor, const char *path)
> +{
> +	struct process_info *pinfo;
> +	struct wl_client *client;
> +
> +	pinfo = zalloc(sizeof *pinfo);
> +	if (!pinfo)
> +		return NULL;
> +
> +	pinfo->path = strdup(path);
> +	if (!pinfo->path)
> +		goto out_free;
> +
> +	client = weston_client_launch(compositor, &pinfo->proc, path,
> +				      process_handle_sigchld);
> +	if (!client)
> +		goto out_str;
> +
> +	return client;
> +
> +out_str:
> +	free(pinfo->path);
> +
> +out_free:
> +	free(pinfo);
> +
> +	return NULL;
> +}
> +
>  static void
>  log_uname(void)
>  {
> diff --git a/src/text-backend.c b/src/text-backend.c
> index ab4667f..743cbc4 100644
> --- a/src/text-backend.c
> +++ b/src/text-backend.c
> @@ -33,6 +33,7 @@
>  #include <time.h>
>  
>  #include "compositor.h"
> +#include "weston.h"
>  #include "text-input-unstable-v1-server-protocol.h"
>  #include "input-method-unstable-v1-server-protocol.h"
>  #include "shared/helpers.h"
> diff --git a/src/weston.h b/src/weston.h
> index 63f47cd..4cfab6c 100644
> --- a/src/weston.h
> +++ b/src/weston.h
> @@ -30,9 +30,33 @@
>  extern "C" {
>  #endif
>  
> +#include "compositor.h"
> +
>  void *
>  load_weston_module(const char *name, const char *entrypoint);
>  
> +struct weston_process;
> +typedef void (*weston_process_cleanup_func_t)(struct weston_process *process,
> +					    int status);
> +
> +struct weston_process {
> +	pid_t pid;
> +	weston_process_cleanup_func_t cleanup;
> +	struct wl_list link;
> +};
> +
> +struct wl_client *
> +weston_client_launch(struct weston_compositor *compositor,
> +		     struct weston_process *proc,
> +		     const char *path,
> +		     weston_process_cleanup_func_t cleanup);
> +
> +struct wl_client *
> +weston_client_start(struct weston_compositor *compositor, const char *path);
> +
> +void
> +weston_watch_process(struct weston_process *process);
> +
>  #ifdef  __cplusplus
>  }
>  #endif
> diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
> index 80fcdf7..362893e 100644
> --- a/tests/ivi_layout-test-plugin.c
> +++ b/tests/ivi_layout-test-plugin.c
> @@ -33,6 +33,7 @@
>  #include <assert.h>
>  
>  #include "src/compositor.h"
> +#include "src/weston.h"
>  #include "weston-test-server-protocol.h"
>  #include "ivi-test.h"
>  #include "ivi-shell/ivi-layout-export.h"
> diff --git a/tests/weston-test.c b/tests/weston-test.c
> index 03e2c54..bda0d91 100644
> --- a/tests/weston-test.c
> +++ b/tests/weston-test.c
> @@ -32,6 +32,7 @@
>  #include <string.h>
>  
>  #include "src/compositor.h"
> +#include "src/weston.h"
>  #include "weston-test-server-protocol.h"
>  
>  #ifdef ENABLE_EGL
> diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
> index b1fd904..e09c6f9 100644
> --- a/xwayland/xwayland.h
> +++ b/xwayland/xwayland.h
> @@ -30,6 +30,7 @@
>  #include <cairo/cairo-xcb.h>
>  
>  #include "compositor.h"
> +#include "weston.h"
>  
>  #define SEND_EVENT_MASK (0x80)
>  #define EVENT_TYPE(event) ((event)->response_type & ~SEND_EVENT_MASK)
> 


More information about the wayland-devel mailing list