[PATCH] linux: Do not call FatalError from xf86CloseConsole

[Peter Hutterer]

On Fri, Oct 16, 2015 at 09:06:49AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 16-10-15 06:41, Peter Hutterer wrote:
> >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.
> 
> True, but then we cannot differentiate between VT_ACTIVATE failing and VT_WAITACTIVE
> failing. Maybe always do a xf86Msg(X_WARNING, ...) from switch_to itself and then
> in addition if switch_to failed, do a FatalError in the open path ?

yep, sounds like a plan.

Cheers,
   Peter