[PATCH xserver 2/9] Remove SIGIO support for input [v3]

[Keith Packard]

Peter Hutterer <peter.hutterer at who-t.net> writes:

Thanks for taking a look at these. This one is less mechanical than I'd
like, so some of the changes aren't obvious. It's definitely good to
review them carefully.

>>  KdNotifyFd(int fd, int ready, void *data)
>>  {
>>      int i = (int) (intptr_t) data;
>> -    OsBlockSIGIO();
>>      (*kdInputFds[i].read)(fd, kdInputFds[i].closure);
>> -    OsReleaseSIGIO();
>
> how comes you don't replace these with input_lock/input_unlock()? if there's
> a specific reason it'd be better to have this in a follow-up patch, I'd
> prefer this patch to be mostly mechanical.

Just that we won't need a lock here in the threaded input case; threaded
input holds the input lock across all input processing. This could go in
a separate patch, but of course with SIGIO removed, we wouldn't need
locking anyways.

If I replaced them with input_lock/input_unlock, I'd just have to remove
those calls when threaded input got added.

>>  static void
>>  KdAddFd(int fd, int i)
>>  {
>> -    struct sigaction act;
>> -    sigset_t set;
>> -
>> -    kdnFds++;
>> -    fcntl(fd, F_SETOWN, getpid());
>>      KdNonBlockFd(fd);
>> -    AddEnabledDevice(fd);
>
>
> it's not clear why you're dropping this bit.

(the fcntl should be self-evident)

The AddEnabledDevice call was not necessary, given the SetNotifyFd call
just below. Was a harmless bug before threaded input as it just whacked
the same mask that SetNotifyFd was going to whack. However, with
threaded input, we don't want the main server select to see the input
devices, so the call needs to be removed.

> same here.

Same reason.

>> -    {FLAG_USE_SIGIO, "UseSIGIO", OPTV_BOOLEAN,
>> -     {0}, FALSE},
>
> not 100% here - if you take this out won't current configs with that option
> now throw an error? should't we be more lenient here?

Good point. Leaving this in seems harmless enough. One wonders if this
value has ever been used...

Do you want me to rearrange the other fixes above to make more sense? Or
is it good enough to have explanation here?

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160516/68a16fa3/attachment.sig>