[PATCH weston] screen-share: Allow fullscreen shell command to be configured

[Jason Ekstrand]

On Thu, May 1, 2014 at 9:00 AM, Andrew Wedgbury <andrew.wedgbury at realvnc.com
> wrote:

> Currently the screen-share module uses a hard-coded command to start the
> fullscreen shell server. This patch causes the module to read the command
> from
> the weston config file (from the "command" key in the "screen-share"
> section).
> The default value remains the same (i.e. to run weston with the RDP
> backend and
> fullscreen shell), but is now located in the weston config file.
>
> As well as allowing the arguments to the fullscreen shell server to be
> changed,
> this also permits an alternative fullscreen shell server to be used if
> required,
> without needing to recompile. Since the command is run as the user running
> weston, this should not pose any additional security risk.
>
> ---
>  src/screen-share.c | 62
> ++++++++++++++++++++++++++++++++++++++++++------------
>  weston.ini.in      |  3 +++
>  2 files changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/src/screen-share.c b/src/screen-share.c
> index 6f60b81..6e9a14f 100644
> --- a/src/screen-share.c
> +++ b/src/screen-share.c
> @@ -32,6 +32,7 @@
>  #include <signal.h>
>  #include <linux/input.h>
>  #include <errno.h>
> +#include <ctype.h>
>
>  #include <wayland-client.h>
>
> @@ -101,6 +102,11 @@ struct ss_shm_buffer {
>         pixman_image_t *pm_image;
>  };
>
> +struct screen_share {
> +       struct weston_compositor *compositor;
> +       char *command;
> +};
> +
>  static void
>  ss_seat_handle_pointer_enter(void *data, struct wl_pointer *pointer,
>                              uint32_t serial, struct wl_surface *surface,
> @@ -982,13 +988,15 @@ shared_output_destroy(struct shared_output *so)
>  }
>
>  static struct shared_output *
> -weston_output_share(struct weston_output *output,
> -                   const char *path, char *const argv[])
> +weston_output_share(struct weston_output *output, const char* command)
>  {
>         int sv[2];
>         char str[32];
>         pid_t pid;
>         sigset_t allsigs;
> +       struct wl_array args;
> +       char **argv;
> +       char *start, *p, **ps;
>
>         if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sv) < 0) {
>                 weston_log("weston_output_share: socketpair failed: %m\n");
> @@ -1025,7 +1033,28 @@ weston_output_share(struct weston_output *output,
>                 snprintf(str, sizeof str, "%d", sv[1]);
>                 setenv("WAYLAND_SERVER_SOCKET", str, 1);
>
> -               execv(path, argv);
> +               wl_array_init(&args);
> +               start = zalloc(strlen(command)+1);
> +               if (start == 0)
> +                       return NULL;
> +               strcpy(start, command);
> +               while (*start) {
> +                       for (p = start; *p && !isspace(*p); p++)
> +                               ;
> +
> +                       ps = wl_array_add(&args, sizeof *ps);
> +                       *ps = start;
> +
> +                       while (*p && isspace(*p))
> +                               *p++ = '\0';
> +
> +                       start = p;
> +               }
> +               ps = wl_array_add(&args, sizeof *ps);
> +               *ps = NULL;
> +
> +               argv = args.data;
> +               execv(argv[0], argv);
>                 weston_log("weston_output_share: exec failed: %m\n");
>                 abort();
>         } else {
> @@ -1056,7 +1085,7 @@ share_output_binding(struct weston_seat *seat,
> uint32_t time, uint32_t key,
>                      void *data)
>  {
>         struct weston_output *output;
> -       const char *path = BINDIR "/weston";
> +       struct screen_share *ss = data;
>
>         if (!seat->pointer) {
>                 weston_log("Cannot pick output: Seat does not have
> pointer\n");
> @@ -1071,23 +1100,28 @@ share_output_binding(struct weston_seat *seat,
> uint32_t time, uint32_t key,
>                 return;
>         }
>
> -       char *const argv[] = {
> -               "weston",
> -               "--backend=rdp-backend.so",
> -               "--shell=fullscreen-shell.so",
> -               "--no-clients-resize",
> -               NULL
> -       };
> -
> -       weston_output_share(output, path, argv);
> +       weston_output_share(output, ss->command);
>

Why are we doing our own command parsing?  Why not just run "sh -c command"
instead of doing our own command-line parsing?  Then we could just do:

       char *const argv[] = {
               "/bin/sh",
               "-c",
               command,
               NULL
       };

 It's much simpler and would allow for proper quoting etc.  Is there a good
reason why we shouldn't do this?

 }
>
>  WL_EXPORT int
>  module_init(struct weston_compositor *compositor,
>             int *argc, char *argv[])
>  {
> +       struct screen_share *ss;
> +       struct weston_config_section *section;
> +
> +       ss = zalloc(sizeof *ss);
> +       if (ss == NULL)
> +               return -1;
> +       ss->compositor = compositor;
> +
> +       section = weston_config_get_section(compositor->config,
> "screen-share",
> +                                           NULL, NULL);
> +
> +       weston_config_section_get_string(section, "command", &ss->command,
> "");
>

Do we want an actual default here rather than just putting something in the
default config file?


> +
>         weston_compositor_add_key_binding(compositor, KEY_S,
>                                           MODIFIER_CTRL | MODIFIER_ALT,
> -                                         share_output_binding,
> compositor);
> +                                         share_output_binding, ss);
>         return 0;
>  }
> diff --git a/weston.ini.in b/weston.ini.in
> index 2c39177..1f216a7 100644
> --- a/weston.ini.in
> +++ b/weston.ini.in
> @@ -65,3 +65,6 @@ path=@libexecdir@/weston-keyboard
>  #constant_accel_factor = 50
>  #min_accel_factor = 0.16
>  #max_accel_factor = 1.0
> +
> +[screen-share]
> +command=@bindir@/weston --backend=rdp-backend.so
> --shell=fullscreen-shell.so --no-clients-resize
> --
> 1.9.2
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140501/c4de702d/attachment.html>