Re: [Utopia] Gnome-VFS backend crashing file chooser dialog

["John (J5) Palmieri" <johnp redhat com>]

On Thu, 2004-06-17 at 16:08, Sjoerd Simons wrote:
> On Thu, Jun 17, 2004 at 12:46:56PM +0200, David Zeuthen wrote:
> > On Wed, Jun 16, 2004 at 04:47:21PM -0400, John (J5) Palmieri wrote:
> > > I was wondering if anyone else is seeing this. Using the latest patch
> > > from Dave it seems that clicking on any of the volumes in the shortcut
> > > window of the file chooser causes a crash.  This has been seen by others
> > > who use my patch packages.  I have been trying to track it down at I
> > > know the crash happens at a strcmp in shorcut_find_position where
> > > base_path is NULL.  This is because the volume data that was populated
> > > into the shortcut's list model became invalid at some point.  I tracked
> > > it down to the fstab and the mtab lists of the vfs daemon somehow
> > > trashing its own data.  Beyond that I can't figure out where the
> > > trashing is actually happening.  Below is a backtrace of the symptom
> > > with the last unknown symbol being the strcmp.  Hopefully someone can
> > > add some insight.  
> > > 
> > 
> > Hi,
> > 
> > I'm out travelling right now, back saturday, but I'll have a look at
> > it when I get back. Unless you beat me to it, that is :-)
> 
> Beat you :).. Well actually i was intrested in the whole gtk filechooser -> vfs
> -> hal link worked and the bug provided a nice challenge to dive into the code
> (apart from annoying me)
> 
> The problem was that the hal patch filters some standard unix filesystems mount
> points and doesn't report them to the vfs. Now the gnome-vfs filechooser
> backend wants the volume for / (which is filtered) and assumes there is one.
> The fact that there isn't leads to a strcmp with a NULL arguments eventually.
> 
> Imho filtering out some paths isn't the right sollution, the user_visible flag
> is ment for this.. So i've removed the filtering and let the user_visible flag
> of drives depends on the hal removable flag. David put a comment somewhere that
> the storage flag is unreliable, but it seems to work fine here...
> 
> Attached patch is to be applied on a gnomevfs patched with davids patch.
> 
> Unfortunately the current hal in debian (need to check cvs) doesn't always
> recognize which volumes are mounted, which leads to the same problem.
> 
>   Sjoerd

Hmm, after looking over your patch in more detail, I don't think we only
want to show removable drives.  We want to show all drives that are not
standard Unix mounts. So any windows drives in /mnt or just a second
storage drive would be fair game.  Attached is a modification of your
patch.  It compiles fine but I have yet to check it.  Will check it in
the morning. 

--
J5

--- gnome-vfs-2.6.0/libgnomevfs/gnome-vfs-hal-mounts.c.rootfix	2004-06-11 18:06:22.000000000 -0400
+++ gnome-vfs-2.6.0/libgnomevfs/gnome-vfs-hal-mounts.c	2004-06-17 21:24:37.000000000 -0400
@@ -608,7 +608,7 @@
 
 /***********************************************************************/
 
-/** This function is used to skip certain volumes/drives we don't
+/** These functions are used to skip or hide certain volumes/drives we don't
  *  want to expose in GnomeVFS.
  *
  */
@@ -617,7 +617,24 @@
 		     GnomeVFSHalVolume *hal_vol,  /* may be NULL */
 		     char *mount_point)
 {
-	/* Skip standard UNIX-like mount points */
+	/* Skip volumes where HAL couldn't figure out the filesystem type */
+	if (hal_vol != NULL) {
+		/* but allow blank discs */
+		if (!(hal_vol->is_disc && hal_vol->disc_is_blank)) {
+			if (hal_vol->fstype == NULL)
+				return TRUE;
+		}
+	}
+
+	return FALSE;
+}
+
+static gboolean
+_hal_check_for_hide (GnomeVFSHalDrive *hal_drive,
+                     GnomeVFSHalVolume *hal_vol,  /* may be NULL */
+                     char *mount_point)
+{
+	/* Hide standard UNIX-like mount points */
 	if (strcmp (mount_point, "/var") == 0 ||
 	    strcmp (mount_point, "/usr") == 0 ||
 	    strcmp (mount_point, "/bin") == 0 ||
@@ -629,15 +646,6 @@
 	    strcmp (mount_point, "/") == 0)
 		return TRUE;
 
-	/* Skip volumes where HAL couldn't figure out the filesystem type */
-	if (hal_vol != NULL) {
-		/* but allow blank discs */
-		if (!(hal_vol->is_disc && hal_vol->disc_is_blank)) {
-			if (hal_vol->fstype == NULL)
-				return TRUE;
-		}
-	}
-
 	return FALSE;
 }
 
@@ -693,6 +701,9 @@
 	/* See if we should skip this */
 	if (_hal_check_for_skip (hal_drive, NULL, mount_point))
 		goto out;
+
+	/* See if we sould hide this */
+	computer_visible = !_hal_check_for_hide (hal_drive, NULL, mount_point);
 	
 	/* see if drive was already added */
 	drive = _gnome_vfs_volume_monitor_find_drive_by_hal_udi (
@@ -845,6 +856,9 @@
 	if (_hal_check_for_skip (hal_drive, hal_vol, mount_point))
 		goto out;
 
+	/* See if we sould hide this */
+	computer_visible = !_hal_check_for_hide (hal_drive, NULL, mount_point);
+
 #ifdef HAL_ONLY_SHOW_MOUNTED_VOLUMES
 	/* we should also show blank discs */
 	if (!hal_vol->is_mounted && !is_blank_disc)