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 <giuliocamu...@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)
> 
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to