subdiff added a comment.
Since there seem to be not many others I'll try to give you reviews for this
and your following patches, but first I've to understand this stuff.
Tell me if the following is right:
Until now the greeter waited for the exit code of the forked process with the
pid `m_pid`. The child process executes the kcheckpass binary via execlp for
pam/etcshadow password validation and exits immediately. The parent process now
reads the requests and results from kcheckpass via socket `m_fd`. It gets
notified when kcheckpass has written something to the socket thanks to
`m_notifier`.
First kcheckpass's pam/etcshadows part asks for the provided password, i.e.
transmits `ConvGetNormal` / `ConvGetHidden`, so `KCheckPass::handleVerify`
writes back the provided password to the socket. kcheckpass can apparently
without having to wait directly read it from the socket. Pam/etcshadow compares
it with the set user password and (until now) exits with the respective success
or fail code.
With your change instead of setting an exit code conv_server writes back one
more time to the socket with the result (i.e. only the enum signaling it with
empty prompt).
INLINE COMMENTS
> authenticator.cpp:221
> return;
> case ConvGetHidden:
> if (!GRecvArr( &arr ))
When you're at it: This seems to do the exact same thing as in `ConvGetNormal`
case. Code duplication?
> authenticator.cpp:270
> if (errno != EINTR) { // This should not happen ...
> cantCheck();
> return;
When we have already received `ConvPutAuthSucceeded` this would mean that we
first emitted `succeeded` and later `failed`, which could lead to problems.
Just delete the whole reapVerify method and end `m_notifier` as well as close
`m_fd` in handleVerify.
REPOSITORY
R133 KScreenLocker
REVISION DETAIL
https://phabricator.kde.org/D4806
To: graesslin, #plasma
Cc: subdiff, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol
