D5825: Include signal.h for kill(), and prototype kqueue/kevent stuff...

Adriaan de Groot noreply at phabricator.kde.org
Wed May 24 08:49:46 UTC 2017


adridg added inline comments.

INLINE COMMENTS

> kcheckpass.c:77
> +#endif
>  
>  #define THROTTLE 3

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)

> kcheckpass.c:384
> +    EV_SET(&keventEvent[1], SIGUSR2, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
> +    int setupResult = kevent(keventQueue, &keventEvent, 2, NULL, 0, NULL);
> +    if (setupResult == -1) {

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.

> kcheckpass.c:419
> +        message("keventData: %d\n",keventData);
> +        if ( keventData == -1 ) {
> +            /* Let's figure this out in the future, shall we */

Leftover debugging code.

> kcheckpass.c:422
> +            message("ABOOOOOOORT? errno=%d\n", errno);
> +        }
> +        else if ( keventData == 0 ) {

message("kevent error %d\n", errno);

> kcheckpass.c:425
> +            /* Do we need to handle timeouts? */
> +            message("kevent timeout\n");
> +        }

Since timeout is NULL, this will not timeout, but wait indefinitely -- presumably like the read() in the HAVE_SIGNALFD_H case will.

> kcheckpass.c:470
> +        } else if ( keventEvent[0].ident == SIGUSR2 ) {
> +            break;
> +#endif

Here, and in the code for SIGUSR1, the check for sending process is missing -- doesn't kevent support that? (Doesn't look like it)

REPOSITORY
  R133 KScreenLocker

REVISION DETAIL
  https://phabricator.kde.org/D5825

To: tcberner, #freebsd, graesslin
Cc: adridg, davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170524/d4e0eb2c/attachment-0001.html>


More information about the Plasma-devel mailing list