Bug 128757

Summary: Serious media:/ regression on FreeBSD
Product: [Unmaintained] kio Reporter: Michael Nottebrock <lofi>
Component: mediaAssignee: Kevin Ottens <ervin>
Status: RESOLVED FIXED    
Severity: normal CC: mueller
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: FreeBSD Ports   
OS: FreeBSD   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: possible fix

Description Michael Nottebrock 2006-06-07 16:42:50 UTC
Version:            (using KDE KDE 3.5.3)
Installed from:    FreeBSD Ports
OS:                FreeBSD

This commit http://websvn.kde.org/branches/KDE/3.5/kdebase/kioslave/media/mediamanager/fstabbackend.cpp?rev=521680&r1=503760&r2=521680
effectively breaks media:/ on FreeBSD - the contents of a mounted drive show up for a few seconds, then vanish.

Reverting the change restores expected behaviour.
Comment 1 Kevin Ottens 2006-06-09 23:10:41 UTC
Dirk: you committed the incriminated code. Could you provide some insight on this commit?
Comment 2 Dirk Mueller 2006-06-10 00:47:20 UTC
Interesting.
Comment 3 Dirk Mueller 2006-06-10 00:48:36 UTC
Created attachment 16540 [details]
possible fix

I've not tested it yet, but I guess this should resolve the bug. 
Could you please verify?
Comment 4 Michael Nottebrock 2006-06-10 17:31:45 UTC
Yes, that patch fixes this bug, but introduces another: After starting KDE, all my desktop device-icons (I have some for my CD devices) are missing now - they appear only once I go to media:/ in Konqueror and mount/browse a cd device from there.
Comment 5 Michael Nottebrock 2006-06-10 17:33:48 UTC
N.B. - these are actually not 'device-icons' but "Links to Devices" created from the kdesktop context menu (Create New -> Link to Device -> ...)
Comment 6 Kevin Ottens 2006-06-10 17:41:00 UTC
Actually these links have nothing to do with kio media... I doubt the behavior you see now has anything to do with Dirk's patch. Are you sure you didn't modify something else?
Comment 7 Michael Nottebrock 2006-06-10 18:52:57 UTC
I'm sorry, I was confused - those icons which are now disappearing on me are indeed 'device-icons', not 'Links to Devices'. I used to use the latter, but switched to the former once media:/ started to work on FreeBSD. :)
Comment 8 Thiago Macieira 2006-06-11 12:36:28 UTC
I can see the same problem on Linux (r548000), fstab-backend without any patch applied: the media applet in Kicker shows empty until I mount something.
Comment 9 Dirk Mueller 2006-06-11 12:52:28 UTC
Thiago: lets not confuse people. that seems to be a different bug. 
Comment 10 Michael Nottebrock 2006-06-11 13:15:37 UTC
Probably, the media applet works okay here.
Comment 11 Dirk Mueller 2006-06-14 11:11:52 UTC
SVN commit 551306 by mueller:

fix media handling regression caused by stat-avoidance patch
BUG:128757


 M  +10 -6     fstabbackend.cpp  
 M  +2 -1      fstabbackend.h  


--- branches/KDE/3.5/kdebase/kioslave/media/mediamanager/fstabbackend.cpp #551305:551306
@@ -144,7 +144,7 @@
 
 void FstabBackend::handleMtabChange(bool allowNotification)
 {
-	QStringList new_mtabIds, new_mtabEntries;
+	QStringList new_mtabIds;
 	KMountPoint::List mtab = KMountPoint::currentMountPoints();
 
 	KMountPoint::List::iterator it = mtab.begin();
@@ -162,18 +162,19 @@
 		   nothing has changed, do not stat the mount point. Avoids
 		   hang if network shares are stalling */
 		QString mtabEntry = dev + "*" + mp + "*" + fs;
-		bool isOldEntry = m_mtabEntries.contains(mtabEntry);
-		new_mtabEntries+=mtabEntry;
-		if (isOldEntry) continue;
+		if(m_mtabEntries.contains(mtabEntry)) {
+		        new_mtabIds += m_mtabEntries[mtabEntry];
+			continue;
+		}
 
 		QString id = generateId(dev, mp);
 		new_mtabIds+=id;
+		m_mtabEntries[mtabEntry] = id;
 
 		if ( !m_mtabIds.contains(id) && m_fstabIds.contains(id) )
 		{
 			QString mime, icon, label;
 			guess(dev, mp, fs, true, mime, icon, label);
-
 			m_mediaList.changeMediumState(id, true, false,
 			                              mime, icon, label);
 		}
@@ -211,6 +212,10 @@
 			QString mp = medium->mountPoint();
 			QString fs = medium->fsType();
 
+
+			QString mtabEntry = dev + "*" + mp + "*" + fs;
+			m_mtabEntries.remove(mtabEntry);
+
 			QString mime, icon, label;
 			guess(dev, mp, fs, false, mime, icon, label);
 
@@ -226,7 +231,6 @@
 	}
 
 	m_mtabIds = new_mtabIds;
-        m_mtabEntries = new_mtabEntries;
 }
 
 void FstabBackend::handleFstabChange(bool allowNotification)
--- branches/KDE/3.5/kdebase/kioslave/media/mediamanager/fstabbackend.h #551305:551306
@@ -23,6 +23,7 @@
 
 #include <qobject.h>
 #include <qstringlist.h>
+#include <qmap.h>
 
 #ifdef Q_OS_FREEBSD
 #include <qtimer.h>
@@ -53,7 +54,7 @@
 
 	bool m_networkSharesOnly;
 	QStringList m_mtabIds;
-        QStringList m_mtabEntries;
+        QMap<QString, QString> m_mtabEntries;
 	QStringList m_fstabIds;
 #ifdef Q_OS_FREEBSD
 	QTimer m_mtabTimer;