Review Request 124413: Enable PAM opening KWallet again

[Martin Klapetek]

> On July 21, 2015, 5:57 p.m., Lamarque Souza wrote:
> > src/runtime/kwalletd/main.cpp, line 113
> > <https://git.reviewboard.kde.org/r/124413/diff/1/?file=386596#file386596line113>
> >
> >     You should use strncmp instead of strcmp.

Why would you think? The whole string is being compared, what good would strncmp do in here?


> On July 21, 2015, 5:57 p.m., Lamarque Souza wrote:
> > src/runtime/kwalletd/main.cpp, line 126
> > <https://git.reviewboard.kde.org/r/124413/diff/1/?file=386596#file386596line126>
> >
> >     You should use strtol() instead of atoi() for better error checking. atoi() does no error checking at all.

...but the code does not check for errors (and does not need to)?


> On July 21, 2015, 5:57 p.m., Lamarque Souza wrote:
> > src/runtime/kwalletd/main.cpp, line 144
> > <https://git.reviewboard.kde.org/r/124413/diff/1/?file=386596#file386596line144>
> >
> >     You should deallocate hash here if it is not null, otherwise you have a memory leak.

I don't think so - the hash is init'ed as NULL; it is set by waitForHash() which either returns NULL (so hash stays NULL) or returns some data, at which point the code will not reach this line (can someone double check).


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124413/#review82770
-----------------------------------------------------------


On July 21, 2015, 5:52 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124413/
> -----------------------------------------------------------
> 
> (Updated July 21, 2015, 5:52 p.m.)
> 
> 
> Review request for KDE Frameworks, Àlex Fiestas and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> This brings back Alex's patch in commit f2fe3e75b4ba12d0f99aa09327059a1865891b14 [1] which allows KWallet to be opened by PAM if kwallet-pam is present.
> 
> http://quickgit.kde.org/?p=kde-runtime.git&a=commit&h=f2fe3e75b4ba12d0f99aa09327059a1865891b14
> 
> 
> Diffs
> -----
> 
>   src/runtime/kwalletd/main.cpp b4e3837 
> 
> Diff: https://git.reviewboard.kde.org/r/124413/diff/
> 
> 
> Testing
> -------
> 
> Logged in, KWallet does not ask for password anymore.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150721/60c2f42f/attachment-0001.html>