Bug 87637 - KFloppy: no user visible error message when fdformat finds an error at verify
Summary: KFloppy: no user visible error message when fdformat finds an error at verify
Status: RESOLVED FIXED
Alias: None
Product: kfloppy
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Bernd Wuebben
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-08-20 21:22 UTC by Nicolas Goutte
Modified: 2005-05-07 16:59 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Goutte 2004-08-20 21:22:10 UTC
Version:            (using KDE KDE 3.3.0)
Installed from:    Compiled From Sources
Compiler:          gcc 3.3.1 -march=pentium2
OS:                Linux

When fdformat finds an error on the floppy, KFloppy returns to the normal dialog without any error.

The problem is that an user might assume that formatting is finished and then wonders that the floppy does not work, as it has not any file system.

Have a nice day!
Comment 1 Nicolas Goutte 2004-09-09 00:40:48 UTC
Problem still remaining.

There are 2 problems:
- fdformat complains with a string having a number, so the current code assumes that the number is a track number. (Not good!)
- when an "action" quits with an error, the user is not informed. (Not good either!)

Have a nice day!
Comment 2 Nicolas Goutte 2005-01-01 17:51:14 UTC
CVS commit by goutte: 

Process two error messages of fdformat
(Especially useful, as one of the errors is typical for a failed verify.)
(Linux only)
CCBUG:87637


  M +17 -2     format.cpp   1.25


--- kdeutils/kfloppy/format.cpp  #1.24:1.25
@@ -483,8 +483,23 @@ void FDFormat::processStdOut(KProcess *,
         DEBUGS(s);
         QRegExp regexp( "([0-9]+)" );
-        if ( regexp.search(s) > -1 )
+        if (s.startsWith("bad data at cyl") || s.startsWith("Problem reading cylinder"))
+        {
+            const int track = regexp.cap(1).toInt();
+            if (track>=0)
         {
+                emit status(i18n("Low-level formatting error at track %1!").arg(track), -1);
+            }
+            else
+            {
+                // This error should not happen
+                emit status(i18n("Low-level formatting error: %1").arg(s), -1);
+            }
+            return;
+        }
+        else if ( regexp.search(s) > -1 )
+        {
+            // Normal track number (formatting or verifying)
             const int p = regexp.cap(1).toInt();
-            if ((p>0) && (p<deviceInfo->tracks))
+            if ((p>=0) && (p<deviceInfo->tracks))
             {
                     emit status(QString::null,


Comment 3 Nicolas Goutte 2005-05-06 16:23:57 UTC
Waldo Bastian has probably fixed this in the revision 39550 of the file kdeutils/kfloppy/format.cpp
Comment 4 Nicolas Goutte 2005-05-06 16:24:52 UTC
I meant revision 395550 
Comment 5 Nicolas Goutte 2005-05-06 17:41:53 UTC
SVN commit 410059 by goutte:

- Try to give useful messages why a program (especially fdformat) has failed
  (Especially allow more than one error message to be passed to the dialog.)
- Be more careful while processing fdformat errors
  (Checking for numbers must be last, as /dev/fd0u1440 contains a number too.)
CCBUG:87637

- Give a better error message if fdformat cannot work because the device
  is mounted. (The work is done only for fdformat now.)
CCBUG:103954


 M  +14 -1     trunk/KDE/kdeutils/kfloppy/floppy.cpp  
 M  +19 -12    trunk/KDE/kdeutils/kfloppy/format.cpp  


--- trunk/KDE/kdeutils/kfloppy/floppy.cpp #410058:410059
@@ -410,6 +410,9 @@
   // formatbutton->setText(i18n("A&bort"));
   setEnabled(false);
 
+        // Erase text box
+        frame->setText( QString::null );
+
 	if (!findDevice())
 	{
 		reset();
@@ -515,7 +518,17 @@
 {
     kdDebug(2002) << "FloppyData::formatStatus: " << s << " : "  << p << endl;
 	if (!s.isEmpty())
-		frame->setText(s);
+        {
+            const QString oldText ( frame->text() );
+            if ( oldText.isEmpty() )
+            {
+                frame->setText( s );
+            }
+            else
+            {
+                frame->setText( oldText + '\n' + s );
+            }
+        }
 
 	if ((0<=p) && (p<=100))
 		progress->setValue(p);
--- trunk/KDE/kdeutils/kfloppy/format.cpp #410058:410059
@@ -332,13 +332,13 @@
 		}
 		else
 		{
-			emit status(i18n("%1 terminated with an error.").arg(theProcessName),100);
+			emit status(i18n("The program %1 terminated with an error.").arg(theProcessName),100);
 			emit done(this,false);
 		}
 	}
 	else
 	{
-		emit status(i18n("%1 terminated abnormally.").arg(theProcessName),100);
+		emit status(i18n("The program %1 terminated abnormally.").arg(theProcessName),100);
 		emit done(this,false);
 	}
 }
@@ -504,16 +504,6 @@
             }
             return;
         }
-        else if ( regexp.search(s) > -1 )
-        {
-            // Normal track number (formatting or verifying)
-            const int p = regexp.cap(1).toInt();
-            if ((p>=0) && (p<deviceInfo->tracks))
-            {
-                    emit status(QString::null,
-                            p * 100 / deviceInfo->tracks);
-            }
-        }
 	else if (s.find("ioctl(FDFMTBEG)")!=-1)
 	{
             emit status (i18n(
@@ -522,11 +512,28 @@
                     "have selected a valid floppy drive."),-1);
             return;
 	}
+        else if (s.find("busy")!=-1) // "Device or resource busy"
+        {
+            emit status(i18n("Device busy!\nPerhaps you need to unmount the floppy first!"),-1);
+            return;
+        }
+        // Be careful to leave "iotcl" as last before checking numbers
         else if (s.find("ioctl")!=-1)
         {
             emit status(i18n("Low-level format error: %1").arg(s),-1);
             return;
         }
+        // Check for numbers at last (as /dev/fd0u1440 has numbers too)
+        else if ( regexp.search(s) > -1 )
+        {
+            // Normal track number (formatting or verifying)
+            const int p = regexp.cap(1).toInt();
+            if ((p>=0) && (p<deviceInfo->tracks))
+            {
+                    emit status(QString::null,
+                            p * 100 / deviceInfo->tracks);
+            }
+        }
 #endif
 	return;
 }
Comment 6 Nicolas Goutte 2005-05-06 17:51:44 UTC
SVN commit 410067 by goutte:

Propagate the "device busy" error to the user (FAT, Ext2, Minix)
(Unfortunately, it is shown two times currently!)
(Linux only)

CCBUG: 103954
CCBUG: 87637


 M  +55 -1     trunk/KDE/kdeutils/kfloppy/format.cpp  
 M  +9 -1      trunk/KDE/kdeutils/kfloppy/format.h  


--- trunk/KDE/kdeutils/kfloppy/format.cpp #410066:410067
@@ -626,11 +626,29 @@
 	}
 }
 
+void FATFilesystem::processStdOut(KProcess *, char *b, int l)
+{
+#ifdef ANY_BSD
+    // ### TODO: do some checks
+#elif defined(ANY_LINUX)
+    QString s ( QString::fromLatin1( b, l ) );
+    kdDebug(KFAREA) << s << endl;
+    if (s.find("mounted file system")!=-1) // "/dev/fd0 contains a mounted file system
+    {
+        emit status(i18n("Floppy is mounted!\nYou need to unmount the floppy first!"),-1);
+        return;
+    }
+    else if (s.find("busy")!=-1) // "Device or resource busy"
+    {
+        emit status(i18n("Device busy!\nPerhaps you need to unmount the floppy first!"),-1);
+        return;
+    }
+#endif
+}
 
 
 
 
-
 #ifdef ANY_BSD
 
 /* static */ QString UFSFilesystem::newfs = QString::null ;
@@ -758,8 +776,28 @@
 	}
 }
 
+void Ext2Filesystem::processStdOut(KProcess *, char *b, int l)
+{
+#ifdef ANY_BSD
+    // ### TODO: do some checks
+#elif defined(ANY_LINUX)
+    QString s ( QString::fromLatin1( b, l ) );
+    kdDebug(KFAREA) << s << endl;
+    if (s.find("mounted")!=-1) // "/dev/fd0 is mounted; will not make a filesystem here!"
+    {
+        emit status(i18n("Floppy is mounted!\nYou need to unmount the floppy first!"),-1);
+        return;
+    }
+    else if (s.find("busy")!=-1) // "Device or resource busy"
+    {
+        emit status(i18n("Device busy!\nPerhaps you need to unmount the floppy first!"),-1);
+        return;
+    }
+#endif
+}
 
 
+
 #endif
 
 #ifdef ANY_LINUX
@@ -833,6 +871,22 @@
 	}
 }
 
+void MinixFilesystem::processStdOut(KProcess *, char *b, int l)
+{
+    QString s ( QString::fromLatin1( b, l ) );
+    kdDebug(KFAREA) << s << endl;
+    if (s.find("mounted")!=-1) // "mkfs.minix: /dev/fd0 is mounted; will not make a filesystem here!"
+    {
+        emit status(i18n("Floppy is mounted!\nYou need to unmount the floppy first!"),-1);
+        return;
+    }
+    else if (s.find("busy")!=-1) // "Device or resource busy"
+    {
+        emit status(i18n("Device busy!\nPerhaps you need to unmount the floppy first!"),-1);
+        return;
+    }
+}
+
 #endif
 
 #include "format.moc"
--- trunk/KDE/kdeutils/kfloppy/format.h #410066:410067
@@ -283,6 +283,9 @@
 	 */	
 	bool configure(bool verify, bool label, const QString &l);
 	
+        /// Parse output
+        virtual void processStdOut(KProcess*, char* b, int l);
+        
 protected:
 	static QString newfs_fat;
 	
@@ -313,6 +316,9 @@
 	
 	/// Same args as FATFilesystem::configure
 	bool configure(bool verify, bool label, const QString &l);
+
+        /// Parse output
+        virtual void processStdOut(KProcess*, char* b, int l);
 	
 protected:
 	static QString newfs;
@@ -358,7 +364,9 @@
 	
 	/// Same args as FATFilesystem::configure
 	bool configure(bool verify, bool label, const QString &l);
-	
+        
+        /// Parse output
+        virtual void processStdOut(KProcess*, char* b, int l);
 protected:
 	static QString newfs;
 	
Comment 7 Nicolas Goutte 2005-05-06 18:44:48 UTC
SVN commit 410082 by goutte:

- avoid that messages from the high-level formatters would be duplicated
- fix the detecting of the track number when fdformat fails verifying
  (Linux only)
CCBUG:87637


 M  +0 -16     trunk/KDE/kdeutils/kfloppy/floppy.cpp  
 M  +2 -3      trunk/KDE/kdeutils/kfloppy/format.cpp  


--- trunk/KDE/kdeutils/kfloppy/floppy.cpp #410081:410082
@@ -444,10 +444,6 @@
 	if ( filesystemComboBox->currentText() == i18n("DOS") )
 	{
 		FATFilesystem *f = new FATFilesystem(this);
-		connect(f,SIGNAL(status(const QString &,int)),
-			this,SLOT(formatStatus(const QString &,int)));
-		connect(f,SIGNAL(done(KFAction *,bool)),
-			this,SLOT(reset()));
 		f->configure(verifylabel->isChecked(),
 			labellabel->isChecked(),
 			lineedit->text());
@@ -463,10 +459,6 @@
 			lineedit->text());
 		if (f)
 		{
-			connect(f,SIGNAL(status(const QString &,int)),
-				this,SLOT(formatStatus(const QString &,int)));
-			connect(f,SIGNAL(done(KFAction *,bool)),
-				this,SLOT(reset()));
 			f->configureDevice(drive,blocks);
 			formatActions->queue(f);
 		}
@@ -480,10 +472,6 @@
 
 		if (f)
 		{
-			connect(f,SIGNAL(status(const QString &,int)),
-				this,SLOT(formatStatus(const QString &,int)));
-			connect(f,SIGNAL(done(KFAction *,bool)),
-				this,SLOT(reset()));
 			f->configureDevice(drive,blocks);
 			formatActions->queue(f);
 		}
@@ -499,10 +487,6 @@
 			lineedit->text());
 		if (f)
 		{
-			connect(f,SIGNAL(status(const QString &,int)),
-				this,SLOT(formatStatus(const QString &,int)));
-			connect(f,SIGNAL(done(KFAction *,bool)),
-				this,SLOT(reset()));
 			f->configureDevice(drive,blocks);
 			formatActions->queue(f);
 		}
--- trunk/KDE/kdeutils/kfloppy/format.cpp #410081:410082
@@ -133,7 +133,6 @@
 
 	if (!success)
 	{
-            // ### TODO: the user must be told that the action has failed.
 		DEBUGS("Action failed.");
 		emit done(this,false);
 		return;
@@ -492,9 +491,9 @@
         QRegExp regexp( "([0-9]+)" );
         if (s.startsWith("bad data at cyl") || s.startsWith("Problem reading cylinder"))
         {
-            const int track = regexp.cap(1).toInt();
-            if (track>=0)
+            if ( regexp.search( s ) > -1 )
             {
+                const int track = regexp.cap(1).toInt();
                 emit status(i18n("Low-level formatting error at track %1!").arg(track), -1);
             }
             else
Comment 8 Nicolas Goutte 2005-05-07 16:59:06 UTC
This works now for "trunk". I will have to see what can be backported while giving sense without new user-visible messages.