[PATCH] linux: Do not call FatalError from xf86CloseConsole

[Peter Hutterer]

On Wed, Oct 14, 2015 at 04:03:54PM +0200, Hans de Goede wrote:
> FatalError ends up calling xf86CloseConsole itself, so calling FatalError
> from within xf86CloseConsole is not a good idea.
> 
> All the other error checking done in xf86CloseConsole uses
> xf86Msg(X_WARNING, ...) except for the switch_to() helper function,
> change things so that switch_to() also uses xf86Msg rather then FatalError
> when called from xf86CloseConsole.
> 
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1269210
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  hw/xfree86/os-support/linux/lnx_init.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c
> index ec06a05..9fec964 100644
> --- a/hw/xfree86/os-support/linux/lnx_init.c
> +++ b/hw/xfree86/os-support/linux/lnx_init.c
> @@ -64,17 +64,25 @@ drain_console(int fd, void *closure)
>  }
>  
>  static void
> -switch_to(int vt, const char *from)
> +switch_to(int vt, Bool is_open)
>  {
>      int ret;
>  
>      SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_ACTIVATE, vt));
> -    if (ret < 0)
> -        FatalError("%s: VT_ACTIVATE failed: %s\n", from, strerror(errno));
> +    if (ret < 0) {
> +        if (is_open)
> +            FatalError("xf86OpenConsole: VT_ACTIVATE failed: %s\n", strerror(errno));
> +        else
> +            xf86Msg(X_WARNING, "xf86CloseConsole: VT_ACTIVATE failed: %s\n", strerror(errno));
> +    }
>  
>      SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_WAITACTIVE, vt));
> -    if (ret < 0)
> -        FatalError("%s: VT_WAITACTIVE failed: %s\n", from, strerror(errno));
> +    if (ret < 0) {
> +        if (is_open)
> +            FatalError("xf86OpenConsole: VT_WAITACTIVE failed: %s\n", strerror(errno));
> +        else
> +            xf86Msg(X_WARNING, "xf86CloseConsole: VT_WAITACTIVE failed: %s\n", strerror(errno));
> +    }
>  }

I think the better approach here be to return the error and handle the
failure in the caller. it's a bit convoluted otherwise.

Cheers,
   Peter

>  
>  #pragma GCC diagnostic push
> @@ -233,7 +241,7 @@ xf86OpenConsole(void)
>              /*
>               * now get the VT.  This _must_ succeed, or else fail completely.
>               */
> -            switch_to(xf86Info.vtno, "xf86OpenConsole");
> +            switch_to(xf86Info.vtno, TRUE);
>  
>              SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_GETMODE, &VT));
>              if (ret < 0)
> @@ -294,7 +302,7 @@ xf86OpenConsole(void)
>      else {                      /* serverGeneration != 1 */
>          if (!xf86Info.ShareVTs && xf86Info.autoVTSwitch) {
>              /* now get the VT */
> -            switch_to(xf86Info.vtno, "xf86OpenConsole");
> +            switch_to(xf86Info.vtno, TRUE);
>          }
>      }
>  }
> @@ -346,7 +354,7 @@ xf86CloseConsole(void)
>           * Perform a switch back to the active VT when we were started
>           */
>          if (activeVT >= 0) {
> -            switch_to(activeVT, "xf86CloseConsole");
> +            switch_to(activeVT, FALSE);
>              activeVT = -1;
>          }
>      }
> -- 
> 2.5.0
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>