Small fixes for execute_kwallet
ClosedPublic

Authored by aacid on May 21 2018, 10:31 AM.

Details

Summary

Don't return on error conditions, exit instead. It's a child process we
need to finish it
Don't use pam_syslog, there's a comment at the beginning saying it
doesn't work, it seems to kind of work for me but some investigation by
Matthias Gerstner seems to show it's doing funny things with the fds
so don't use it.

Diff Detail

Repository
R107 KWallet PAM Integration
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
aacid created this revision.May 21 2018, 10:31 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 21 2018, 10:31 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
aacid requested review of this revision.May 21 2018, 10:31 AM
fvogt accepted this revision.May 21 2018, 11:31 AM
This revision is now accepted and ready to land.May 21 2018, 11:31 AM
anthonyfieroni added inline comments.
pam_kwallet.c
392–393

fullSocket is not free'd.

399–401

Here also.

fullSocket was malloc'd by the parent process. free()-ing it is pointless.

fullSocket was malloc'd by the parent process. free()-ing it is pointless.

it's not pointless, it's "unneeded" since the child process will exit on the "cleanup" goto and then all its resources should be freed anyway by the OS, *but* it is the correct thing to do, the fact that the memory was allocated by the parent process doesn't mean the memory still needs freeing in the child process too, i.e. fork basically "mallocs" the memory again (simplified version of what is happening). Example: at https://paste.kde.org/psqeol0uc

aacid updated this revision to Diff 34615.May 21 2018, 10:15 PM

free fullSocket

@fvogt @anthonyfieroni happy with this new revision?

fvogt accepted this revision.May 23 2018, 6:47 AM
This revision was automatically updated to reflect the committed changes.