graesslin added a comment.
In https://phabricator.kde.org/D4806#96684, @subdiff wrote: > 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`. yes, the exit code was the auth result. exit code 0 means auth success, everything else failure. > 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`. yes > 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. yes > 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). yes > EDIT: Follow up question: In the current version at which point in the execution is the reapVerify method called at all? handleVerify is only connected to the activated method of QSocketNotifier and I assume that kcheckpass writes to the socket in the end and handleVerify is called one last time with `GRecvInt( &ret )` being false. But I don't see any more writes to the socket in kcheckpass.c and for example checkpass_shadow.c after the initial query for the provided password. reapVerify seems to me to be only if the communication horribly breaks. I did not fully understand the code and what it was doing. Part is to wait for the auth result, part is further error checking. In the long run I want to replace this by a nice QProcess, but one step at a time. This change is actually only a preparation step for the follow up which has the long running kcheckpass. INLINE COMMENTS > subdiff wrote in authenticator.cpp:221 > When you're at it: This seems to do the exact same thing as in > `ConvGetNormal` case. Code duplication? not unlikely. That code is old and got carried around. 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
