kcheckpass: Add support in for non-Linux platforms via kevent.
ClosedPublic
Actions

Authored by tcberner on May 12 2017, 7:50 PM.

Details

Reviewers

graesslin
kde-mac
davidedmundson

Group Reviewers

FreeBSD

Commits

R133:c2fd8526c523: kcheckpass: Add support in for non-Linux platforms via kevent.

Summary

signalfd() is a Linux specific api: SIGNALFD(2):

CONFORMING TO         top

signalfd() and signalfd4() are Linux-specific.

FreeBSD and OSX and others can use the widely available kevent/kqueue api for a similar effect.

We let the kernel notify us via kevent() [1], if there is a SIGUSR1 or SIGUSR2, and then fetch the signal using sigwaitinfo() [2].

[1] https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2
[2] https://www.freebsd.org/cgi/man.cgi?query=sigwaitinfo&sektion=2

It seems to work just fine on FreeBSD.

Diff Detail

Repository

R133 KScreenLocker

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

tcberner created this revision.May 12 2017, 7:50 PM

Restricted Application added a project: Plasma. · View Herald TranscriptMay 12 2017, 7:50 PM

Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

tcberner edited the summary of this revision. (Show Details)May 12 2017, 7:52 PM

tcberner retitled this revision from Include signal.h for kill. to Include signal.h for kill(), and prototype kqueue/kevent stuff... .

but now that it is all here already, let's just call this a prototype, for someone who knows C/kevent/kqueue to fix.

So what doesn't work?

In D5825#109093, @davidedmundson wrote:

but now that it is all here already, let's just call this a prototype, for someone who knows C/kevent/kqueue to fix.

So what doesn't work?

I have not yet had time to test it at all :)

The recent change that uses signalfd() is a linuxism. Possibly a glibism as well. This patch introduces an alternate path for systems without signalfd(), using kevent() to do the Same Thing (tm) .. but as tcberner says, it's not been tested. Consider it a prototype.

adridg added inline comments.May 24 2017, 8:49 AM

kcheckpass/kcheckpass.c
77	Possibly add #if !(HAVE_SIGNALFD_H_ \|\| HAVE_EVENT_H) #error #endif to double-check that it's one or the other (reflecting the check in CMake)
384	This should use the signals set up in the mask signalMask; if it is possible to iterate over them, you should do so. Otherwise changes above (e.g. an additional call to sigaddset()) are easy to forget here.
423	Leftover debugging code.
426	message("kevent error %d\n", errno);
429	Since timeout is NULL, this will not timeout, but wait indefinitely -- presumably like the read() in the HAVE_SIGNALFD_H case will.
498	Here, and in the code for SIGUSR1, the check for sending process is missing -- doesn't kevent support that? (Doesn't look like it)

I want to look at this some more this weekend.

kcheckpass/kcheckpass.c
77	That should not be necessary, as cmake will fail if neither is present.
498	Yes, I'm aware of its absence, however, I could not yet find out how to get that data.

I can successfully unlock the lockscreen, with this code .

Use sigwaitinfo() to get information on the sending-pid.

tcberner retitled this revision from Include signal.h for kill(), and prototype kqueue/kevent stuff... to kcheckpass: Add support in for non-Linux platforms via kevent..May 28 2017, 6:49 AM

tcberner edited the summary of this revision. (Show Details)

tcberner added a reviewer: kde-mac.May 29 2017, 7:56 AM

tcberner marked 2 inline comments as done.

In D5825#109698, @tcberner wrote:

I have not yet had time to test it at all :)

Do you actually intend to test this on Mac? I laud and applaud the initiative but I see only one way that the package will ever get used on Mac - providing KWin as a standalone alternative *X11* window manager for use under XQuartz (knowing that XQuartz currently doesn't support compositing). And that's not possible with stock Qt (not until some of us team up to polish my PoC Qt/XCB-on-Mac patches and submit them for upstream incorporation; the Qt guys are likely to accept them as it'd fit with their policy and there might be a "market").

FWIW, funny coincidence, I did manage to build kscreenlocker 5.9.3 on Mac last week, simply #ifdeffing-out parts of the code that where Linux specific or required an X11 KWindowSystem build (I just build the plugin). The screenlocker blacks out the screen but lacking keyboard event support it had to be killed. Or just quit via the native Mac menu item, which kind of defeats the purpose of a screenlocker ;)

FWIW2: if you feel like starting a crusade against linuxisms, please do start with libwayland. It has a number of those (signalfd, epoll) which stand in the way of bringing Wayland to Mac, and there'd definitely be a market for that.

tcberner edited the summary of this revision. (Show Details)May 29 2017, 8:29 AM

Do you actually intend to test this on Mac?

Nope, I don't have a Mac :) -- but I hoped, that there were some Mac users who could check it on their end too (as you also have kqueue/kevent) -- but I did not know that most of the stuff is not yet ported for Mac.

In D5825#112442, @tcberner wrote:

Do you actually intend to test this on Mac?

Nope, I don't have a Mac :) -- but I hoped, that there were some Mac users who could check it on their end too (as you also have kqueue/kevent) -- but I did not know that most of the stuff is not yet ported for Mac.

You ought to be able to set up a VM (or build a dual-boot Hackintosh ;)) but there are several reasons why this endeavour is a bit futile on Mac. The most important of which are that Qt and KF5 code in general assume that X11 isn't available (or shouldn't be used) on Mac, and a prevalent apparent attitude among many (or just the most vocal) Plasma devs that Plasma is off-limits on Mac.

But even without those obstacles, the XQuartz X11 environment runs as an "embedded server" inside a standard Aqua session and I don't know of any way to run any other kind of session. Of course you can run an X11 screensaver and/or screenlocker but what's the point if you already have the same functionality in the main session? In fact, the same *but full-featured* functionality; I strongly doubt that an X11 screenlocker will (nor should) provide features to suspend, shutdown/restart the system or login as another user.

Already getting updates through kde-mac, no need to receive everything twice.

OS X is not a supported Plasma platform, BSD is. Whether it works there or not too important.

You can put this in the 5.10 branch for 5.10.1 which is out pretty soon after 5.10

This revision is now accepted and ready to land.May 29 2017, 9:10 AM

Closed by commit R133:c2fd8526c523: kcheckpass: Add support in for non-Linux platforms via kevent. (authored by tcberner). · Explain WhyMay 29 2017, 6:07 PM

This revision was automatically updated to reflect the committed changes.

In D5825#112464, @davidedmundson wrote:

OS X is not a supported Plasma platform, BSD is.

Just for future reference: OS X doesn't have sigwaitinfo() (or the cited alternative).

In D5825#112682, @rjvbb wrote:

In D5825#112464, @davidedmundson wrote:

OS X is not a supported Plasma platform, BSD is.

Just for future reference: OS X doesn't have sigwaitinfo() (or the cited alternative).

Nevertheless OS X is not supported by kscreenlocker and will never be.

Nevertheless OS X is not supported by kscreenlocker and will never be.

You got it all wrong, OS X doesn't support kscreenlocking, gna!

(If I were still a fanboy I'd add that it's way too kool to support running most kinds of plasma O:^) )

Revision Contents
Changeset List

			Path	Packages
M			CMakeLists.txt (14 lines)
M			config-kscreenlocker.h.cmake (2 lines)
M			greeter/authenticator.cpp (1 line)
M			kcheckpass/kcheckpass.c (101 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	14451	f287b3a		May 12 2017, 7:50 PM	★	★
Diff 2	14906	f287b3a	Use sigwaitinfo() to get information on the sending-pid.	May 28 2017, 6:46 AM	★	★
Diff 3	14951	e4484f0	R133:c2fd8526c5238085f30169fb1891ba56ee1cbda8	May 29 2017, 6:06 PM	★	★