Review Request: Fix some memory leaks in the Fonts Installer KCM

[Craig Drummond]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1567/#review2301
-----------------------------------------------------------


Changes are good - except:
1. Use QStyle(parent) - and not setParent() - matches the rest of the code better.
2. No need for fcConfig variable just change FcInitLoadConfigAndFonts() to FcInitReinitialize()

I'll make these changes to trunk and the KDE4.3 branch.

ps. this kcm does still have a maintainer - me :-)


svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/kcmfontinst/FontFilterProxyStyle.cpp
<http://reviewboard.kde.org/r/1567/#comment1625>

    Please just pass 'parent' to the QStyle constructor.



svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/lib/FcEngine.h
<http://reviewboard.kde.org/r/1567/#comment1626>

    Please, use the same coding standard as the rest of the file. In this case it would be 'itsFcConfig'



svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/lib/FcEngine.cpp
<http://reviewboard.kde.org/r/1567/#comment1623>

    Again, adhere to the coding style of the file you are editing. In this case single line 'if' is not followed by brackets.



svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/lib/FcEngine.cpp
<http://reviewboard.kde.org/r/1567/#comment1624>

    By looking at fontconfig's code (specifically) fcinit.c - it would appear the this class should be just calling FcInitReinitialize()
    
    FcInitReinitialize() internally calls FcInitLoadConfigAndFonts() and sets the returned config to be the default one.
    
    Therefore, I think FcInitReinitialize() is what should be used - and then there is no need for the fcConfig variable.


- Craig


On 2009-09-11 00:47:15, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1567/
> -----------------------------------------------------------
> 
> (Updated 2009-09-11 00:47:15)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> I'm not confident about this one (about the Fc* function calls), and as I don't know if this KCM has a current maintainer to ping, I'm leaving the patch here to discuss. 
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/lib/FcEngine.h 1021986 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/lib/FcEngine.cpp 1021986 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/viewpart/FontPreview.cpp 1021986 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/kcontrol/kfontinst/kcmfontinst/FontFilterProxyStyle.cpp 1021986 
> 
> Diff: http://reviewboard.kde.org/r/1567/diff
> 
> 
> Testing
> -------
> 
> It fixes several memory leaks easily found running the KCM using valgrind. 
> 
> 
> Thanks,
> 
> Darío
> 
>