[PATCH] linux: Do not call FatalError from xf86CloseConsole

[Hans de Goede]

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 ?

Regards,

Hans