Review Request 126410: Fix FILE* leak in kwalletd

Martin Klapetek martin.klapetek at gmail.com
Fri Jan 8 20:01:08 UTC 2016



> On Jan. 8, 2016, 7:50 p.m., Martin Klapetek wrote:
> > Heads-up: this commit actually broke kwallet when opened from pam;
> > kwallet is basically blocked forever and all requests time out.
> > 
> > I'll try to break down this patch to pinpoint which change it was
> > exactly.
> 
> Martin Klapetek wrote:
>     This is the change that breaks kwallet:
>     
>     ```diff
>     +    fclose(s3);
>     +
>     ```
>     
>     I'm not entirely sure (yet) why though.
> 
> Christoph Cullmann wrote:
>     Hi, don't you need to do f = fdopen (dup(filehandle)) to have not fclose(f) close the handle, too, and not only the FILE *?

You're right; `FILE *s3 = fdopen(dup(s2), "r");` /does/ fix the issue.


- Martin


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


On Dec. 18, 2015, 11:50 p.m., Michael Pyne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126410/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2015, 11:50 p.m.)
> 
> 
> Review request for KDE Frameworks, Martin Klapetek and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> Fix a leak of a `FILE*` in kwalletd, spotted by Coverity (CID 1335090), and a couple of minor fixes/cleanups since I was there.
> 
> 1. No need to memset a `char[]`, you can let compiler default-initialize it (and likely even be able to move the whole array into .bss).
> 2. The chop after `fgets` can easily be confused for a null terminator check, and `fgets` isn't required to return a newline anyways, so add sanity checking (which helps document better at the same time).
> 
> 
> Diffs
> -----
> 
>   src/runtime/kwalletd/main.cpp db8d55c 
> 
> Diff: https://git.reviewboard.kde.org/r/126410/diff/
> 
> 
> Testing
> -------
> 
> Everything compiles. Not much beyond done.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160108/b87f273d/attachment.html>


More information about the Kde-frameworks-devel mailing list