[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: [Patch] Fix undeleted "static QFont *_font" in kglobalsettings.cpp
From: "Friedrich W. H. Kossebau" <kossebau () kde ! org>
Date: 2008-12-04 13:16:15
Message-ID: 200812041416.16466.kossebau () kde ! org
[Download RAW message or body]
Hi,
while putting Okteta into valgrind's memcheck I found that it looks like all
the static fonts defined in
kdelibs/kdeui/kernel/kglobalsettings.cpp
are not cleaned up on program exit.
For practice, good code behaviour and to reduce the ouput of memcheck I would
like to change that, and here is my first^Wsecond approach:
The attached patch puts all the file-globally declared objects (fonts,
mousesettings) into a container class, which creates them lazyly as before,
but now deletes them on destruction. That container is accessed via
inline GlobalSettingsData* GlobalSettingsData::self()
{
K_GLOBAL_STATIC(GlobalSettingsData, s_self)
return s_self;
}
This should ensure correct construction and destruction at the lifetime of the
process, right?
I decided against reusing KGlobalSettings::Private as the container, because
the old font functions did not create the KGlobalSettings singleton but could
be used without it. I could not oversee the sideeffects by changing that. Can
you?
For KDE5 the API of KGlobalSettings might want to be fixed (static vs.
non-static), I added this as comment.
Additional bonus of the patch: shrinks all repeating font object creation code
into a single one and collects the default data into a table.
Two best practice questions, for handling the "QFont* mFonts[FontCount]" :
1. How to set an array of pointers all to 0 most efficiently (mFonts)?
2. How to mark the number of entries in an enum, so adding one more increases
that number (FontCount) automatically?
Cheers
Friedrich
--
Okteta - KDE 4 Hex Editor - http://utils.kde.org/projects/okteta
["deleteStaticDataOfKGlobalSettings2.patch" (text/x-diff)]
Index: kdelibs/kdeui/kernel/kglobalsettings.cpp
===================================================================
--- kdelibs/kdeui/kernel/kglobalsettings.cpp (Revision 891841)
+++ kdelibs/kdeui/kernel/kglobalsettings.cpp (Arbeitskopie)
@@ -73,19 +73,76 @@
#include <stdlib.h>
#include <kconfiggroup.h>
-static QFont *_generalFont = 0;
-static QFont *_fixedFont = 0;
-static QFont *_toolBarFont = 0;
-static QFont *_menuFont = 0;
-static QFont *_windowTitleFont = 0;
-static QFont *_taskbarFont = 0;
-static QFont *_largeFont = 0;
-static QFont *_smallestReadableFont = 0;
+
//static QColor *_buttonBackground = 0;
static KGlobalSettings::GraphicEffects _graphicEffects = KGlobalSettings::NoEffects;
-static KGlobalSettings::KMouseSettings *s_mouseSettings = 0;
+// KDE5: merge this with KGlobalSettings::Private
+// also think to make all methods static and not expose an object,
+// making KGlobalSettings rather a namespace
+class GlobalSettingsData
+{
+ public:
+ enum FontTypes
+ {
+ GeneralFont=0,
+ FixedFont=1,
+ ToolbarFont=2,
+ MenuFont=3,
+ WindowTitleFont=4,
+ TaskbarFont=5,
+ SmallestReadableFont=6,
+ FontCount=7
+ };
+ protected:
+ GlobalSettingsData();
+ public:
+ ~GlobalSettingsData();
+
+ public:
+ static GlobalSettingsData* self();
+
+ public: // access, is not const due to caching
+ QFont font( FontTypes fontType );
+ QFont largeFont( const QString& text );
+ KGlobalSettings::KMouseSettings& mouseSettings();
+
+ public:
+ void dropFontSettingsCache();
+ void dropMouseSettingsCache();
+
+ protected:
+ QFont* mFonts[FontCount];
+ QFont* mLargeFont;
+ KGlobalSettings::KMouseSettings* mMouseSettings;
+};
+
+GlobalSettingsData::GlobalSettingsData()
+ : mLargeFont( 0 ),
+ mMouseSettings( 0 )
+{
+ // FIXME: best style to set an array of pointers all to 0?
+ for( int i=0; i<FontCount; ++i )
+ mFonts[i] = 0;
+}
+
+GlobalSettingsData::~GlobalSettingsData()
+{
+ for( int i=0; i<FontCount; ++i )
+ delete mFonts[i];
+ delete mLargeFont;
+
+ delete mMouseSettings;
+}
+
+inline GlobalSettingsData* GlobalSettingsData::self()
+{
+ K_GLOBAL_STATIC(GlobalSettingsData, s_self)
+ return s_self;
+}
+
+
class KGlobalSettings::Private
{
public:
@@ -116,14 +173,6 @@
void applyCursorTheme();
/**
- * drop cached values for fonts
- */
- static void rereadFontSettings();
- /**
- * drop cached values for mouse settings
- */
- static void rereadMouseSettings();
- /**
* drop cached values for settings that aren't in any of the previous groups
*/
static void rereadOtherSettings();
@@ -311,138 +360,115 @@
return g.readEntry( "allowDefaultBackgroundImages", \
KDE_DEFAULT_ALLOW_DEFAULT_BACKGROUND_IMAGES ); }
-QFont KGlobalSettings::generalFont()
+struct FontData
{
- if (_generalFont)
- return *_generalFont;
+ const char* ConfigGroupKey;
+ const char* ConfigKey;
+ const char* FontName;
+ int Size;
+ int Weight;
+ QFont::StyleHint StyleHint;
+};
+// NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
+static const char GeneralId[] = "General";
+static const char DefaultFont[] = "Sans Serif";
#ifdef Q_WS_MAC
- _generalFont = new QFont("Lucida Grande", 13);
-#else
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _generalFont = new QFont("Sans Serif", 10);
+static const char DefaultMacFont[] = "Lucida Grande";
#endif
- _generalFont->setStyleHint(QFont::SansSerif);
- KConfigGroup g( KGlobal::config(), "General" );
- *_generalFont = g.readEntry("font", *_generalFont);
-
- return *_generalFont;
-}
-
-QFont KGlobalSettings::fixedFont()
+static const FontData DefaultFontData[] =
{
- if (_fixedFont)
- return *_fixedFont;
-
#ifdef Q_WS_MAC
- _fixedFont = new QFont("Monaco", 10);
+ { GeneralId, "font", DefaultMacFont, 13, -1, QFont::SansSerif },
+ { GeneralId, "fixed", "Monaco", 10, -1, QFont::TypeWriter },
+ { GeneralId, "toolBarFont", DefaultMacFont, 11, -1, QFont::SansSerif },
+ { GeneralId, "menuFont", DefaultMacFont, 13, -1, QFont::SansSerif },
#else
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _fixedFont = new QFont("Monospace", 10);
+ { GeneralId, "font", DefaultFont, 10, -1, QFont::SansSerif },
+ { GeneralId, "fixed", "Monospace", 10, -1, QFont::TypeWriter },
+ { GeneralId, "toolBarFont", DefaultFont, 8, -1, QFont::SansSerif },
+ { GeneralId, "menuFont", DefaultFont, 10, -1, QFont::SansSerif },
#endif
- _fixedFont->setStyleHint(QFont::TypeWriter);
+ { "WM", "activeFont", DefaultFont, 9, QFont::Bold, \
QFont::SansSerif },// inconsistency + { GeneralId, "taskbarFont", \
DefaultFont, 10, -1, QFont::SansSerif }, + { GeneralId, "smallestReadableFont", \
DefaultFont, 8, -1, QFont::SansSerif } +};
+QFont GlobalSettingsData::font( FontTypes fontType )
+{
+ QFont* cachedFont = mFonts[fontType];
- KConfigGroup g( KGlobal::config(), "General" );
- *_fixedFont = g.readEntry("fixed", *_fixedFont);
+ if (!cachedFont)
+ {
+ const FontData& fontData = DefaultFontData[fontType];
+ cachedFont = new QFont( fontData.FontName, fontData.Size, fontData.Weight );
+ cachedFont->setStyleHint( fontData.StyleHint );
- return *_fixedFont;
+ KConfigGroup configGroup( KGlobal::config(), fontData.ConfigGroupKey );
+ *cachedFont = configGroup.readEntry( fontData.ConfigKey, *cachedFont );
+
+ mFonts[fontType] = cachedFont;
+ }
+
+ return *cachedFont;
}
+QFont KGlobalSettings::generalFont()
+{
+ return GlobalSettingsData::self()->font( GlobalSettingsData::GeneralFont );
+}
+QFont KGlobalSettings::fixedFont()
+{
+ return GlobalSettingsData::self()->font( GlobalSettingsData::FixedFont );
+}
QFont KGlobalSettings::toolBarFont()
{
- if(_toolBarFont)
- return *_toolBarFont;
-
-#ifdef Q_WS_MAC
- _toolBarFont = new QFont("Lucida Grande", 11);
-#else
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _toolBarFont = new QFont("Sans Serif", 8);
-#endif
- _toolBarFont->setStyleHint(QFont::SansSerif);
-
- KConfigGroup g( KGlobal::config(), "General" );
- *_toolBarFont = g.readEntry("toolBarFont", *_toolBarFont);
-
- return *_toolBarFont;
+ return GlobalSettingsData::self()->font( GlobalSettingsData::ToolbarFont );
}
-
QFont KGlobalSettings::menuFont()
{
- if(_menuFont)
- return *_menuFont;
-
-#ifdef Q_WS_MAC
- _menuFont = new QFont("Lucida Grande", 13);
-#else
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _menuFont = new QFont("Sans Serif", 10);
-#endif
- _menuFont->setStyleHint(QFont::SansSerif);
-
- KConfigGroup g( KGlobal::config(), "General" );
- *_menuFont = g.readEntry("menuFont", *_menuFont);
-
- return *_menuFont;
+ return GlobalSettingsData::self()->font( GlobalSettingsData::MenuFont );
}
-
QFont KGlobalSettings::windowTitleFont()
{
- if(_windowTitleFont)
- return *_windowTitleFont;
-
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _windowTitleFont = new QFont("Sans Serif", 9, QFont::Bold);
- _windowTitleFont->setStyleHint(QFont::SansSerif);
-
- KConfigGroup g( KGlobal::config(), "WM" );
- *_windowTitleFont = g.readEntry("activeFont", *_windowTitleFont); // \
inconsistency
-
- return *_windowTitleFont;
+ return GlobalSettingsData::self()->font( GlobalSettingsData::WindowTitleFont );
}
-
QFont KGlobalSettings::taskbarFont()
{
- if(_taskbarFont)
- return *_taskbarFont;
-
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _taskbarFont = new QFont("Sans Serif", 10);
- _taskbarFont->setStyleHint(QFont::SansSerif);
-
- KConfigGroup g( KGlobal::config(), "General" );
- *_taskbarFont = g.readEntry("taskbarFont", *_taskbarFont);
-
- return *_taskbarFont;
+ return GlobalSettingsData::self()->font( GlobalSettingsData::TaskbarFont );
}
+QFont KGlobalSettings::smallestReadableFont()
+{
+ return GlobalSettingsData::self()->font( \
GlobalSettingsData::SmallestReadableFont ); +}
-QFont KGlobalSettings::largeFont(const QString &text)
+QFont GlobalSettingsData::largeFont(const QString &text)
{
QFontDatabase db;
QStringList fam = db.families();
// Move a bunch of preferred fonts to the front.
- if (fam.removeAll("Arial")>0)
- fam.prepend("Arial");
- if (fam.removeAll("Sans Serif")>0)
- fam.prepend("Sans Serif");
- if (fam.removeAll("Verdana")>0)
- fam.prepend("Verdana");
- if (fam.removeAll("Tahoma")>0)
- fam.prepend("Tahoma");
- if (fam.removeAll("Lucida Sans")>0)
- fam.prepend("Lucida Sans");
- if (fam.removeAll("Lucidux Sans")>0)
- fam.prepend("Lucidux Sans");
- if (fam.removeAll("Nimbus Sans")>0)
- fam.prepend("Nimbus Sans");
- if (fam.removeAll("Gothic I")>0)
- fam.prepend("Gothic I");
+ // most preferred last
+ static const char* PreferredFontNames[] = {
+ "Arial",
+ "Sans Serif",
+ "Verdana",
+ "Tahoma",
+ "Lucida Sans",
+ "Lucidux Sans",
+ "Nimbus Sans",
+ "Gothic I"
+ };
+ for( unsigned int i=0; i<sizeof(PreferredFontNames); ++i )
+ {
+ const QString fontName (PreferredFontNames[i]);
+ if (fam.removeAll(fontName)>0)
+ fam.prepend(fontName);
+ }
- if (_largeFont)
- fam.prepend(_largeFont->family());
+ if (mLargeFont)
+ fam.prepend(mLargeFont->family());
for(QStringList::ConstIterator it = fam.constBegin();
it != fam.constEnd(); ++it)
@@ -469,84 +495,65 @@
continue;
font.setPointSize(48);
- _largeFont = new QFont(font);
- return *_largeFont;
+ mLargeFont = new QFont(font);
+ return *mLargeFont;
}
}
- _largeFont = new QFont(KGlobalSettings::generalFont());
- _largeFont->setPointSize(48);
- return *_largeFont;
+ mLargeFont = new QFont( font(GeneralFont) );
+ mLargeFont->setPointSize(48);
+ return *mLargeFont;
}
-
-QFont KGlobalSettings::smallestReadableFont()
+QFont KGlobalSettings::largeFont( const QString& text )
{
- if(_smallestReadableFont)
- return *_smallestReadableFont;
-
- // NOTE: keep in sync with kdebase/workspace/kcontrol/fonts/fonts.cpp
- _smallestReadableFont = new QFont("Sans Serif", 8);
- _smallestReadableFont->setStyleHint(QFont::SansSerif);
-
- KConfigGroup g( KGlobal::config(), "General" );
- *_smallestReadableFont = g.readEntry("smallestReadableFont", \
*_smallestReadableFont);
-
- return *_smallestReadableFont;
+ return GlobalSettingsData::self()->largeFont( text );
}
-void KGlobalSettings::Private::rereadFontSettings()
+void GlobalSettingsData::dropFontSettingsCache()
{
- delete _generalFont;
- _generalFont = 0L;
- delete _fixedFont;
- _fixedFont = 0L;
- delete _menuFont;
- _menuFont = 0L;
- delete _toolBarFont;
- _toolBarFont = 0L;
- delete _windowTitleFont;
- _windowTitleFont = 0L;
- delete _taskbarFont;
- _taskbarFont = 0L;
- delete _smallestReadableFont;
- _smallestReadableFont = 0L;
+ for( int i=0; i<FontCount; ++i )
+ {
+ delete mFonts[i];
+ mFonts[i] = 0;
+ }
+ delete mLargeFont;
}
-KGlobalSettings::KMouseSettings & KGlobalSettings::mouseSettings()
+KGlobalSettings::KMouseSettings& GlobalSettingsData::mouseSettings()
{
- if ( ! s_mouseSettings )
+ if (!mMouseSettings)
{
- s_mouseSettings = new KMouseSettings;
- KMouseSettings & s = *s_mouseSettings; // for convenience
+ mMouseSettings = new KGlobalSettings::KMouseSettings;
+ KGlobalSettings::KMouseSettings& s = *mMouseSettings; // for convenience
#ifndef Q_WS_WIN
KConfigGroup g( KGlobal::config(), "Mouse" );
QString setting = g.readEntry("MouseButtonMapping");
if (setting == "RightHanded")
- s.handed = KMouseSettings::RightHanded;
+ s.handed = KGlobalSettings::KMouseSettings::RightHanded;
else if (setting == "LeftHanded")
- s.handed = KMouseSettings::LeftHanded;
+ s.handed = KGlobalSettings::KMouseSettings::LeftHanded;
else
{
#ifdef Q_WS_X11
// get settings from X server
// This is a simplified version of the code in input/mouse.cpp
// Keep in sync !
- s.handed = KMouseSettings::RightHanded;
+ s.handed = KGlobalSettings::KMouseSettings::RightHanded;
unsigned char map[20];
int num_buttons = XGetPointerMapping(QX11Info::display(), map, 20);
if( num_buttons == 2 )
{
if ( (int)map[0] == 1 && (int)map[1] == 2 )
- s.handed = KMouseSettings::RightHanded;
+ s.handed = KGlobalSettings::KMouseSettings::RightHanded;
else if ( (int)map[0] == 2 && (int)map[1] == 1 )
- s.handed = KMouseSettings::LeftHanded;
+ s.handed = KGlobalSettings::KMouseSettings::LeftHanded;
}
else if( num_buttons >= 3 )
{
if ( (int)map[0] == 1 && (int)map[2] == 3 )
- s.handed = KMouseSettings::RightHanded;
+ s.handed = KGlobalSettings::KMouseSettings::RightHanded;
else if ( (int)map[0] == 3 && (int)map[2] == 1 )
- s.handed = KMouseSettings::LeftHanded;
+ s.handed = KGlobalSettings::KMouseSettings::LeftHanded;
}
#else
// FIXME: Implement on other platforms
@@ -556,16 +563,23 @@
}
#ifdef Q_WS_WIN
//not cached
- s_mouseSettings->handed = (GetSystemMetrics(SM_SWAPBUTTON) ? \
KMouseSettings::LeftHanded : KMouseSettings::RightHanded); + \
mMouseSettings->handed = (GetSystemMetrics(SM_SWAPBUTTON) ? + \
KGlobalSettings::KMouseSettings::LeftHanded : + \
KGlobalSettings::KMouseSettings::RightHanded); #endif
- return *s_mouseSettings;
+ return *mMouseSettings;
}
+// KDE5: make this a const return?
+KGlobalSettings::KMouseSettings & KGlobalSettings::mouseSettings()
+{
+ return GlobalSettingsData::self()->mouseSettings();
+}
-void KGlobalSettings::Private::rereadMouseSettings()
+void GlobalSettingsData::dropMouseSettingsCache()
{
#ifndef Q_WS_WIN
- delete s_mouseSettings;
- s_mouseSettings = 0L;
+ delete mMouseSettings;
+ mMouseSettings = 0L;
#endif
}
@@ -786,7 +800,7 @@
case FontChanged:
KGlobal::config()->reparseConfiguration();
- KGlobalSettings::Private::rereadFontSettings();
+ GlobalSettingsData::self()->dropFontSettingsCache();
kdisplaySetFont();
break;
@@ -795,7 +809,7 @@
rereadOtherSettings();
SettingsCategory category = static_cast<SettingsCategory>(arg);
if (category == SETTINGS_MOUSE) {
- KGlobalSettings::Private::rereadMouseSettings();
+ GlobalSettingsData::self()->dropMouseSettingsCache();
}
propagateSettings(category);
break;
@@ -920,18 +934,22 @@
void KGlobalSettings::Private::kdisplaySetFont()
{
if (qApp && qApp->type() == QApplication::GuiClient) {
- QApplication::setFont(KGlobalSettings::generalFont());
- QApplication::setFont(KGlobalSettings::menuFont(), "QMenuBar");
- QApplication::setFont(KGlobalSettings::menuFont(), "QMenu");
- QApplication::setFont(KGlobalSettings::menuFont(), "KPopupTitle");
- QApplication::setFont(KGlobalSettings::toolBarFont(), "QToolBar");
+ GlobalSettingsData* data = GlobalSettingsData::self();
+ QApplication::setFont( data->font(GlobalSettingsData::GeneralFont) );
+ const QFont menuFont = data->font( GlobalSettingsData::MenuFont );
+ QApplication::setFont( menuFont, "QMenuBar" );
+ QApplication::setFont( menuFont, "QMenu" );
+ QApplication::setFont( menuFont, "KPopupTitle" );
+ QApplication::setFont( data->font(GlobalSettingsData::ToolbarFont), \
"QToolBar" ); +
#if 0
// "patch" standard QStyleSheet to follow our fonts
Q3StyleSheet* sheet = Q3StyleSheet::defaultSheet();
- sheet->item (QLatin1String("pre"))->setFontFamily \
(KGlobalSettings::fixedFont().family());
- sheet->item (QLatin1String("code"))->setFontFamily \
(KGlobalSettings::fixedFont().family());
- sheet->item (QLatin1String("tt"))->setFontFamily \
(KGlobalSettings::fixedFont().family()); + const QFont fixedFont = data->font( \
GlobalSettingsData::FixedFont ); + sheet->item \
(QLatin1String("pre"))->setFontFamily (fixedFont.family()); + sheet->item \
(QLatin1String("code"))->setFontFamily (fixedFont.family()); + sheet->item \
(QLatin1String("tt"))->setFontFamily (fixedFont.family()); #endif
emit q->kdisplayFontChanged();
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic