[systemd-devel] [PATCH 2/3] logind: session: set_controller should fail if prepare_vt fails

[David Herrmann]

Hi

On Fri, Aug 8, 2014 at 8:45 PM, Olivier Brunel <jjk at jjacky.com> wrote:
> If controllers can expect logind to have "prepared" the VT (e.g. set it to
> graphics mode, etc) then TakeControl() should fail if said preparation
> failed (and session_restore_vt() was called).
> ---
>  src/login/logind-session.c | 15 +++++++++------
>  src/login/logind-session.h |  2 +-
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/login/logind-session.c b/src/login/logind-session.c
> index 905e73f..3f4e177 100644
> --- a/src/login/logind-session.c
> +++ b/src/login/logind-session.c
> @@ -1003,14 +1003,14 @@ static int session_vt_fn(sd_event_source *source, const struct signalfd_siginfo
>          return 0;
>  }
>
> -void session_prepare_vt(Session *s) {
> +int session_prepare_vt(Session *s) {
>          int vt, r;
>          struct vt_mode mode = { 0 };
>          sigset_t mask;
>
>          vt = session_open_vt(s);
>          if (vt < 0)
> -                return;
> +                return vt;

This doesn't work. session_open_vt() returns -1 if the session has no
VT attached. You need to return 0 in that case, though:

if (s->vtnr < 1)
        return 0;
vt = session_open_vt(s);
if (vt < 0)
        return vt;

Thanks
David

>
>          r = fchown(vt, s->user->uid, -1);
>          if (r < 0)
> @@ -1042,11 +1042,12 @@ void session_prepare_vt(Session *s) {
>          if (r < 0)
>                  goto error;
>
> -        return;
> +        return 0;
>
>  error:
>          log_error("cannot mute VT %u for session %s (%d/%d)", s->vtnr, s->id, r, errno);
>          session_restore_vt(s);
> +        return r;
>  }
>
>  void session_restore_vt(Session *s) {
> @@ -1123,8 +1124,6 @@ int session_set_controller(Session *s, const char *sender, bool force) {
>                  return r;
>          }
>
> -        session_swap_controller(s, t);
> -
>          /* When setting a session controller, we forcibly mute the VT and set
>           * it into graphics-mode. Applications can override that by changing
>           * VT state after calling TakeControl(). However, this serves as a good
> @@ -1133,7 +1132,11 @@ int session_set_controller(Session *s, const char *sender, bool force) {
>           * exits.
>           * If logind crashes/restarts, we restore the controller during restart
>           * or reset the VT in case it crashed/exited, too. */
> -        session_prepare_vt(s);
> +        r = session_prepare_vt(s);
> +        if (r < 0)
> +                return r;
> +
> +        session_swap_controller(s, t);
>
>          return 0;
>  }
> diff --git a/src/login/logind-session.h b/src/login/logind-session.h
> index e62b76d..2ab3182 100644
> --- a/src/login/logind-session.h
> +++ b/src/login/logind-session.h
> @@ -172,7 +172,7 @@ SessionClass session_class_from_string(const char *s) _pure_;
>  const char *kill_who_to_string(KillWho k) _const_;
>  KillWho kill_who_from_string(const char *s) _pure_;
>
> -void session_prepare_vt(Session *s);
> +int session_prepare_vt(Session *s);
>  void session_restore_vt(Session *s);
>
>  bool session_is_controller(Session *s, const char *sender);
> --
> 2.0.4
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel