D12937: Drop privileges when reading the salt file

[Rolf Eike Beer]

dakon added inline comments.

INLINE COMMENTS

> aacid wrote in pam_kwallet.c:730
> I can check the return code but does it matter? we'll write into the pipe anyway, be it 0 or whatever we read, if it's not the correct stuff the other process will fail anyway so i don't really see the need to complicate this code more than it already is with another extra if.

Just don't write anything to the pipe, this is easily detectable at the read end. And it will not cause the reader to try to work with random data and fail in a random way, but it will just return a read error.

> aacid wrote in pam_kwallet.c:749
> we're assuming status 0 and != 0 in other places of the code, i'll suggest to also fix this separately in a new patch

Yes please.

> aacid wrote in pam_kwallet.c:750
> the code i'm replacing didn't check for null, really y don't see the need, the code is half checking for null and half not, if you feel very strongly about checking for null maybe we can do it in a different patch and make sure all mallocs check for null?

I don't see a benefit in introducing new bugs just to keep a style. And if the code does it right at some places this one should also just start right.

REPOSITORY
  R107 KWallet PAM Integration

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

To: aacid, dakon
Cc: dakon, mgerstner, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180527/fd00582a/attachment.html>