Hidden KDED desktop file crashing systemsettings - where to fix?

[Jonathan Marten]

> On 11/02/2010 09:26 AM, Jonathan Marten wrote:
>> c) use a KServiceTypeTrader query (which hopefully ignores hidden
>> desktop files) instead of listing them.

Thanks to Sebastian and David for your comments.  I've recoded
KDEDConfig::load() and KDEDConfig::save() to use a sycoca query
instead of listing the files, and tested this with systemsettings.
Patch follows (there is no reviewboard for kdebase), is this OK to
commit?

Possibly the "if ( KDesktopFile::isDesktopFile( ... ) )" is redundant,
but I've tried not to change the structure of the original code too
much.  If anyone thinks this could be cut out, then I'll happily do that.

Regards
  Jonathan

---------------------------------------------------------------------------
Index: kdebase/runtime/kcontrol/kded/kcmkded.cpp
===================================================================
--- kdebase/runtime/kcontrol/kded/kcmkded.cpp	(revision 1191229)
+++ kdebase/runtime/kcontrol/kded/kcmkded.cpp	(working copy)
@@ -35,7 +35,7 @@
 #include <kdialog.h>
 #include <kmessagebox.h>
 #include <kservice.h>
-#include <kstandarddirs.h>
+#include <kservicetypetrader.h>
 
 #include <KPluginFactory>
 #include <KPluginLoader>
@@ -175,46 +175,45 @@
 	_lvStartup->clear();
 	_lvLoD->clear();
 
-	QStringList files;
-	KGlobal::dirs()->findAllResources( "services",
-			QLatin1String( "kded/*.desktop" ),
-			KStandardDirs::Recursive | KStandardDirs::NoDuplicates,
-			files );
-
+        KService::List offers = KServiceTypeTrader::self()->query( "KDEDModule" );
 	QTreeWidgetItem* treeitem = 0L;
-	for ( QStringList::ConstIterator it = files.constBegin(); it != files.constEnd(); ++it ) {
-        kDebug() << *it;
+	for ( KService::List::const_iterator it = offers.constBegin();
+              it != offers.constEnd(); ++it)
+        {
+                QString servicePath = (*it)->entryPath();
+                kDebug() << servicePath;
 
-		if ( KDesktopFile::isDesktopFile( *it ) ) {
-			KDesktopFile file( "services", *it );
-			Q_ASSERT( file.desktopGroup().readEntry("X-KDE-ServiceTypes") == "KDEDModule" );
+		if ( KDesktopFile::isDesktopFile( servicePath ) ) {
+			const KDesktopFile file( "services", servicePath );
+                        const KConfigGroup grp = file.desktopGroup();
+			Q_ASSERT( grp.readEntry("X-KDE-ServiceTypes") == "KDEDModule" );
 			// The logic has to be identical to Kded::initModules.
 			// They interpret X-KDE-Kded-autoload as false if not specified
 			//                X-KDE-Kded-load-on-demand as true if not specified
-			if ( file.desktopGroup().readEntry("X-KDE-Kded-autoload", false) ) {
+			if ( grp.readEntry("X-KDE-Kded-autoload", false) ) {
 				treeitem = new QTreeWidgetItem();
-				treeitem->setCheckState( StartupUse, autoloadEnabled(&kdedrc, *it) ? Qt::Checked : Qt::Unchecked );
+				treeitem->setCheckState( StartupUse, autoloadEnabled(&kdedrc, file.name()) ? Qt::Checked : Qt::Unchecked );
 				treeitem->setText( StartupService, file.readName() );
 				treeitem->setText( StartupDescription, file.readComment() );
 				treeitem->setText( StartupStatus, NOT_RUNNING );
-				if (file.desktopGroup().hasKey("X-KDE-DBus-ModuleName")) {
-					treeitem->setData( StartupService, LibraryRole, file.desktopGroup().readEntry("X-KDE-DBus-ModuleName") );
+				if (grp.hasKey("X-KDE-DBus-ModuleName")) {
+					treeitem->setData( StartupService, LibraryRole, grp.readEntry("X-KDE-DBus-ModuleName") );
 				} else {
 					kWarning() << "X-KDE-DBUS-ModuleName not set for module " << file.readName();
-					treeitem->setData( StartupService, LibraryRole, file.desktopGroup().readEntry("X-KDE-Library") );
+					treeitem->setData( StartupService, LibraryRole, grp.readEntry("X-KDE-Library") );
 				}
 				_lvStartup->addTopLevelItem( treeitem );
 			}
-			else if ( file.desktopGroup().readEntry("X-KDE-Kded-load-on-demand", true) ) {
+			else if ( grp.readEntry("X-KDE-Kded-load-on-demand", true) ) {
 				treeitem = new QTreeWidgetItem();
 				treeitem->setText( OnDemandService, file.readName() );
 				treeitem->setText( OnDemandDescription, file.readComment() );
 				treeitem->setText( OnDemandStatus, NOT_RUNNING );
-				if (file.desktopGroup().hasKey("X-KDE-DBus-ModuleName")) {
-					treeitem->setData( OnDemandService, LibraryRole, file.desktopGroup().readEntry("X-KDE-DBus-ModuleName") );
+				if (grp.hasKey("X-KDE-DBus-ModuleName")) {
+					treeitem->setData( OnDemandService, LibraryRole, grp.readEntry("X-KDE-DBus-ModuleName") );
 				} else {
 					kWarning() << "X-KDE-DBUS-ModuleName not set for module " << file.readName();
-					treeitem->setData( OnDemandService, LibraryRole, file.desktopGroup().readEntry("X-KDE-Library") );
+					treeitem->setData( OnDemandService, LibraryRole, grp.readEntry("X-KDE-Library") );
 				}
 				_lvLoD->addTopLevelItem( treeitem );
 			}
@@ -237,24 +236,21 @@
 }
 
 void KDEDConfig::save() {
-	QStringList files;
-	KGlobal::dirs()->findAllResources( "services",
-			QLatin1String( "kded/*.desktop" ),
-			KStandardDirs::Recursive | KStandardDirs::NoDuplicates,
-			files );
-
 	KConfig kdedrc("kdedrc", KConfig::NoGlobals);
 
-	for ( QStringList::ConstIterator it = files.constBegin(); it != files.constEnd(); ++it ) {
+        KService::List offers = KServiceTypeTrader::self()->query( "KDEDModule" );
+        for ( KService::List::const_iterator it = offers.constBegin();
+              it != offers.constEnd(); ++it)
+        {
+                QString servicePath = (*it)->entryPath();
+                kDebug() << servicePath;
 
-		if ( KDesktopFile::isDesktopFile( *it ) ) {
+		if ( KDesktopFile::isDesktopFile( servicePath ) ) {
+			const KDesktopFile file( "services", servicePath );
+                        const KConfigGroup grp = file.desktopGroup();
+			if (grp.readEntry("X-KDE-Kded-autoload", false)){
 
-                        KConfig _file( *it, KConfig::NoGlobals, "services"  );
-                        KConfigGroup file(&_file, "Desktop Entry");
-
-			if (file.readEntry("X-KDE-Kded-autoload", false)){
-
-				QString libraryName = file.readEntry( "X-KDE-Library" );
+				QString libraryName = grp.readEntry( "X-KDE-Library" );
 				int count = _lvStartup->topLevelItemCount();
 				for( int i = 0; i < count; ++i )
 				{
@@ -262,7 +258,7 @@
 					if ( treeitem->data( StartupService, LibraryRole ).toString() == libraryName )
 					{
 						// we found a match, now compare and see what changed
-						setAutoloadEnabled( &kdedrc, *it, treeitem->checkState( StartupUse ) == Qt::Checked);
+						setAutoloadEnabled( &kdedrc, servicePath, treeitem->checkState( StartupUse ) == Qt::Checked);
 						break;
 					}
 				}

---------------------------------------------------------------------------

-- 
Jonathan Marten                         http://www.keelhaul.demon.co.uk
Twickenham, UK                          jjm2 at keelhaul.demon.co.uk