[PATCH weston v2 1/2] launcher: don't try to switch to the current vt

[Pekka Paalanen]

On Wed, 29 Jun 2016 11:57:11 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
> 
> v2: this was split from the following commit
> 
>  libweston/launcher-direct.c        | 12 ++++++++++++
>  libweston/launcher-impl.h          |  1 +
>  libweston/launcher-logind.c        |  8 ++++++++
>  libweston/launcher-util.c          |  7 ++++++-
>  libweston/launcher-weston-launch.c | 12 ++++++++++++
>  5 files changed, 39 insertions(+), 1 deletion(-)

Hi,

this patch is not actually checking the current VT, it is checking
Weston's VT. That is, you want to avoid Weston switching to Weston's VT.

Under normal circumstances Weston's VT is also the current VT when
Weston is handling keyboard, but in faulty cases it might not be true.
E.g. I think fbdev is still broken with VT switching, so can't say what
would happen there.

Hence I would change the summary to "don't try to switch to weston's VT".

I suppose this patch is needed for the following patch to not trigger
no-op callbacks, and there was no other badness this would fix?

> diff --git a/libweston/launcher-direct.c b/libweston/launcher-direct.c
> index 29d9c28..0ff99a4 100644
> --- a/libweston/launcher-direct.c
> +++ b/libweston/launcher-direct.c
> @@ -305,6 +305,17 @@ launcher_direct_destroy(struct weston_launcher *launcher_base)
>  	free(launcher);
>  }
>  
> +static int
> +launcher_direct_get_vt(struct weston_launcher *base)
> +{
> +	struct launcher_direct *launcher = wl_container_of(base, launcher, base);
> +	struct stat s;
> +	if (fstat(launcher->tty, &s) < 0)
> +		return -1;
> +
> +	return minor(s.st_rdev);
> +}

This looks good.

> +
>  struct launcher_interface launcher_direct_iface = {
>  	launcher_direct_connect,
>  	launcher_direct_destroy,
> @@ -312,4 +323,5 @@ struct launcher_interface launcher_direct_iface = {
>  	launcher_direct_close,
>  	launcher_direct_activate_vt,
>  	launcher_direct_restore,
> +	launcher_direct_get_vt,
>  };
> diff --git a/libweston/launcher-impl.h b/libweston/launcher-impl.h
> index 742721b..0bc3245 100644
> --- a/libweston/launcher-impl.h
> +++ b/libweston/launcher-impl.h
> @@ -34,6 +34,7 @@ struct launcher_interface {
>  	void (* close) (struct weston_launcher *launcher, int fd);
>  	int (* activate_vt) (struct weston_launcher *launcher, int vt);
>  	void (* restore) (struct weston_launcher *launcher);
> +	int (* get_vt) (struct weston_launcher *launcher);

Documenting these would be nice...

>  };
>  
>  struct weston_launcher {
> diff --git a/libweston/launcher-logind.c b/libweston/launcher-logind.c
> index f755ec3..4680402 100644
> --- a/libweston/launcher-logind.c
> +++ b/libweston/launcher-logind.c
> @@ -829,6 +829,13 @@ launcher_logind_destroy(struct weston_launcher *launcher)
>  	free(wl);
>  }
>  
> +static int
> +launcher_logind_get_vt(struct weston_launcher *launcher)
> +{
> +	struct launcher_logind *wl = wl_container_of(launcher, wl, base);
> +	return wl->vtnr;
> +}

Ok.

> +
>  struct launcher_interface launcher_logind_iface = {
>  	launcher_logind_connect,
>  	launcher_logind_destroy,
> @@ -836,4 +843,5 @@ struct launcher_interface launcher_logind_iface = {
>  	launcher_logind_close,
>  	launcher_logind_activate_vt,
>  	launcher_logind_restore,
> +	launcher_logind_get_vt,
>  };
> diff --git a/libweston/launcher-util.c b/libweston/launcher-util.c
> index 03b9d63..5fbfdcf 100644
> --- a/libweston/launcher-util.c
> +++ b/libweston/launcher-util.c
> @@ -97,8 +97,13 @@ switch_vt_binding(struct weston_keyboard *keyboard,
>  		  uint32_t time, uint32_t key, void *data)
>  {
>  	struct weston_compositor *compositor = data;
> +	struct weston_launcher *launcher = compositor->launcher;
> +	int vt = key - KEY_F1 + 1;
>  
> -	weston_launcher_activate_vt(compositor->launcher, key - KEY_F1 + 1);
> +	if (vt == launcher->iface->get_vt(launcher))
> +		return;
> +
> +	weston_launcher_activate_vt(launcher, vt);
>  }

Ok.

>  
>  WL_EXPORT void
> diff --git a/libweston/launcher-weston-launch.c b/libweston/launcher-weston-launch.c
> index ad919f1..b74975a 100644
> --- a/libweston/launcher-weston-launch.c
> +++ b/libweston/launcher-weston-launch.c
> @@ -290,6 +290,17 @@ launcher_weston_launch_destroy(struct weston_launcher *launcher_base)
>  	free(launcher);
>  }
>  
> +static int
> +launcher_weston_launch_get_vt(struct weston_launcher *base)
> +{
> +	struct launcher_weston_launch *launcher = wl_container_of(base, launcher, base);
> +	struct stat s;
> +	if (fstat(launcher->tty, &s) < 0)
> +		return -1;
> +
> +	return minor(s.st_rdev);
> +}

The same as for launcher_direct, ok.

> +
>  struct launcher_interface launcher_weston_launch_iface = {
>  	launcher_weston_launch_connect,
>  	launcher_weston_launch_destroy,
> @@ -297,4 +308,5 @@ struct launcher_interface launcher_weston_launch_iface = {
>  	launcher_weston_launch_close,
>  	launcher_weston_launch_activate_vt,
>  	launcher_weston_launch_restore,
> +	launcher_weston_launch_get_vt,
>  };

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Not sure if this needs rebasing, but since I think you would revise the
second patch, please re-send this one too as rebased.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161125/0285e723/attachment.sig>