Re: Patch: supermount support

[Frederic Crozat <fcrozat mandrakesoft com>]

Le jeu 04/07/2002 à 15:38, Alexander Larsson a écrit :
> On 1 Jul 2002, Frederic Crozat wrote:
> 
> > Ok, here is patch for supermount support (bug 48090)
> > 
> > It is a bit ugly, due to supermount nature => when "umounting"
> > supermount devices, only eject command is started..
> > 
> > Comments (or OK to commit) are welcome..
> 
> --- libnautilus-private/nautilus-volume-monitor.c	28 Jun 2002 10:59:06 -0000	1.131
> +++ libnautilus-private/nautilus-volume-monitor.c	1 Jul 2002 12:30:26 -0000
> @@ -493,7 +493,8 @@
>  	/* Use "owner" or "user" or "users" as our way of determining a removable volume */
>  	if (hasmntopt (ent, "user") != NULL
>  	    || hasmntopt (ent, "users") != NULL
> -	    || hasmntopt (ent, "owner") != NULL) {
> +	    || hasmntopt (ent, "owner") != NULL
> +	    || eel_strcmp ("supermount", ent->mnt_type) == 0) {
>  		return TRUE;
>  	}
>  #endif
> @@ -563,7 +564,16 @@
>  #elif defined (HAVE_MNTENT_H)
>  	while ((ent = getmntent (file)) != NULL) {
>  		if (has_removable_mntent_options (ent)) {
> +#if defined (HAVE_HASMNTOPT)
> +			if (eel_strcmp ("supermount", ent->mnt_type) == 0) {
> +				volume = create_volume (eel_str_strip_substring_and_after (hasmntopt(ent, "dev"), ",") + strlen("dev="), 
> +							ent->mnt_dir);
> +			} else {
> +#endif
> 
> 
> This leaks the return value of eel_str_strip_substring_and_after. It also 
> assumes that any substring "dev" will be on the form dev=something, while 
> not actually looing for the "=". 

Fixed..

> 
>  			volume = create_volume (ent->mnt_fsname, ent->mnt_dir);
> 
> this line is wrongly indented.

Fixed..

> 
> +#if defined (HAVE_HASMNTOPT)
> +			}
> +#endif
>  			volumes = finish_creating_volume_and_prepend
>  				(monitor, volume, ent->mnt_type, volumes);
>  		}
> @@ -617,7 +627,10 @@
>  	}
>  #elif defined (HAVE_MNTENT_H)
>  	while ((ent = getmntent (file)) != NULL) {
> -		if (strcmp (volume->device_path, ent->mnt_fsname) == 0
> +		if (((strcmp (volume->device_path, ent->mnt_fsname) == 0) 
> +		    || (strcmp (ent->mnt_type, "supermount") == 0 && 
> +			strcmp (volume->device_path, eel_str_strip_substring_and_after (hasmntopt (ent, "dev"), ",") + strlen ("dev=")) == 0))
> +		    && (strcmp (volume->mount_path, ent->mnt_dir) == 0)
>  		    && has_removable_mntent_options (ent)) {
>  			removable = TRUE;
>  			break;
> 
> Same here as above. This code should probably be broken out into a 
> function. Also, why did you add the strcmp (volume->mount_path, 
> ent->mnt_dir) == 0 check?

Fixed (add "entry_is_supermounted_volume" function).

The strcmp(volume->mount_path, ent->mnt_dir) is needed because several
supermount mount points can appear to mount the same device (often, the
"none" device) and without this check, you see /proc/bus/usb, /dev/pts,
/dev/shm on your desktop, because they are wrongly seen as removable
devices (same as the supermounted one)..

 
> @@ -1027,6 +1040,20 @@
>                          device_path = eel_string_list_nth (list, 0);
>                          mount_path = eel_string_list_nth (list, 1);
>                          file_system_type_name = eel_string_list_nth (list, 2);
> + 			/* For supermount, search info in removable list */
> + 			if (eel_strcmp ("supermount", file_system_type_name) == 0) {
> + 				GList * list;
> 
> Variable declaration go at the start of the function.

Fixed..

> + 				g_free (device_path);
> 
> Free this when you find a match instead, to avoid a segfault if we didn't 
> find the path ib removable_volumes.

Fixed..

> + 				list = monitor->details->removable_volumes;
> + 				while (list) {
> + 					volume = (NautilusVolume *) list->data;
> + 					if (eel_strcmp (mount_path, volume->mount_path) == 0) {
> + 						device_path = g_strdup (volume->device_path);
> + 						break;
> + 					}
> + 					list = list->next;
> + 				}				
> + 			}
>                          volume = create_volume (device_path, mount_path);
>  			if (eel_string_list_get_length (list) >= 4 &&
>  			    option_list_has_option (eel_string_list_peek_nth (list, 3), MNTOPT_RO))
> @@ -1566,6 +1593,15 @@
>  #else
>         name = mount_point;
>  #endif
> +
> +       /* Don't run mount/umount on supermount mount point, it is useless */
> +       if ((volume->file_system_type != NULL) 
> +	   && (strcmp (volume->file_system_type->name, "supermount") == 0)) {
> +	       if (volume->device_type == NAUTILUS_DEVICE_CDROM_DRIVE) {
> +		       eject_device (volume->device_path);
> +	       }
> +	       return;
> +       }
> 
> Why are you only ejecting cdroms? floppys on some hardware (e.g. macs) 
> support eject.

Because I was using the same logic as in mount_volume_deactivate : you
changed it very recently and it didn't catch the change..

I've changed it by modifying mount_unmount_callback..

And I've also found a bug in eject_device : we shouldn't g_free (path)
since it is a paramater which is freed in mount_unmount_callback...

I've fixed that in the patch..
         
>         if (should_mount) {
>                 command = find_command (MOUNT_COMMAND);
> --- src/file-manager/fm-desktop-icon-view.c	28 Jun 2002 10:59:09 -0000	1.183
> +++ src/file-manager/fm-desktop-icon-view.c	1 Jul 2002 12:30:26 -0000
> @@ -377,7 +377,12 @@
>  	 */	   
>  	index = 1;
>  			
> -	volume_name = nautilus_volume_get_name (volume);	
> +	volume_name = nautilus_volume_get_name (volume);
> +	if ((strcmp(volume_name,_("Unknown")) == 0) 
> +	    && ((nautilus_volume_get_device_type (volume) == NAUTILUS_DEVICE_CDROM_DRIVE)
> +	    || (nautilus_volume_get_device_type (volume) == NAUTILUS_DEVICE_AUDIO_CD))) {
> +		volume_name = g_strdup(_("CD-ROM"));
> +	}		
>  	
> This hack should be in nautilus_volume_get_name(), or even better 
> volume->volume_name could be set to the right thing when the volume is 
> constructed.
> 	
>  	uri_path = g_strdup_printf ("%s/%s", desktop_directory, volume_name);		
>  	uri = gnome_vfs_uri_new (uri_path);

Fixed in mount_volume_make_cdrom_name.. BTW, we should try to launch
gnome-cd when CD type is AUDIO_CD since the cdda support for nautilus is
disabled..

New patch attached..

-- 
Frédéric Crozat
MandrakeSoft

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/nautilus/ChangeLog,v
retrieving revision 1.5410
diff -u -r1.5410 ChangeLog
--- ChangeLog	5 Jul 2002 10:50:48 -0000	1.5410
+++ ChangeLog	5 Jul 2002 14:40:38 -0000
@@ -1,3 +1,19 @@
+2002-07-05  Frederic Crozat  <fcrozat mandrakesoft com>
+
+	* libnautilus-private/filesystem-attributes.xml:
+	* libnautilus-private/nautilus-volume-monitor.c:
+	(has_removable_mntent_options), (get_removable_volumes),
+	(entry_is_supermounted_volume), (volume_is_removable),
+	(mount_volume_make_cdrom_name), (get_mount_list),
+	(mount_unmount_callback),
+	(nautilus_volume_monitor_mount_unmount_removable),
+	(finish_creating_volume):
+	Add support for supermount.
+
+	* libnautilus-private/nautilus-volume-monitor.c:
+	(eject_device):
+	Fix crash when eject media.
+
 2002-07-05  Alexander Larsson  <alexl redhat com>
 
 	* src/nautilus-window-manage-views.c:
Index: libnautilus-private/filesystem-attributes.xml
===================================================================
RCS file: /cvs/gnome/nautilus/libnautilus-private/filesystem-attributes.xml,v
retrieving revision 1.6
diff -u -r1.6 filesystem-attributes.xml
--- libnautilus-private/filesystem-attributes.xml	25 Jun 2002 05:22:05 -0000	1.6
+++ libnautilus-private/filesystem-attributes.xml	5 Jul 2002 14:40:38 -0000
@@ -21,6 +21,7 @@
     <filesystem name="proc"     _default_volume_name="System Volume"/>
     <filesystem name="reiserfs" _default_volume_name="ReiserFS Linux Volume" trash="yes"/>
     <filesystem name="smbfs"    _default_volume_name="Windows Shared Volume" trash="yes"/>
+    <filesystem name="supermount"      _default_volume_name="SuperMount Volume" />
     <filesystem name="udf"      _default_volume_name="DVD Volume"/>
     <filesystem name="ufs"      _default_volume_name="Solaris/BSD Volume" trash="yes"/>
     <filesystem name="udfs"     _default_volume_name="Udfs Solaris Volume" trash="yes"/>
Index: libnautilus-private/nautilus-volume-monitor.c
===================================================================
RCS file: /cvs/gnome/nautilus/libnautilus-private/nautilus-volume-monitor.c,v
retrieving revision 1.131
diff -u -r1.131 nautilus-volume-monitor.c
--- libnautilus-private/nautilus-volume-monitor.c	28 Jun 2002 10:59:06 -0000	1.131
+++ libnautilus-private/nautilus-volume-monitor.c	5 Jul 2002 14:40:38 -0000
@@ -220,6 +220,8 @@
 								 int                        *fd);
 static void            nautilus_volume_free                     (NautilusVolume             *volume);
 static void            nautilus_file_system_type_free           (NautilusFileSystemType     *type);
+static gboolean        entry_is_supermounted_volume             (const MountTableEntry *ent, 
+								 const NautilusVolume *volume);
 
 #ifdef HAVE_CDDA
 static gboolean        locate_audio_cd                          (void);
@@ -493,7 +495,8 @@
 	/* Use "owner" or "user" or "users" as our way of determining a removable volume */
 	if (hasmntopt (ent, "user") != NULL
 	    || hasmntopt (ent, "users") != NULL
-	    || hasmntopt (ent, "owner") != NULL) {
+	    || hasmntopt (ent, "owner") != NULL
+	    || eel_strcmp("supermount", ent->mnt_type) == 0) {
 		return TRUE;
 	}
 #endif
@@ -522,6 +525,7 @@
 	GList *volumes;
 	MountTableEntry *ent;
 	NautilusVolume *volume;
+	char * fs_opt;
 #ifdef HAVE_SYS_MNTTAB_H
         MountTableEntry ent_storage;
 #endif
@@ -563,7 +567,20 @@
 #elif defined (HAVE_MNTENT_H)
 	while ((ent = getmntent (file)) != NULL) {
 		if (has_removable_mntent_options (ent)) {
-			volume = create_volume (ent->mnt_fsname, ent->mnt_dir);
+#if defined (HAVE_HASMNTOPT)
+
+			if (eel_strcmp("supermount", ent->mnt_type) == 0) {
+				fs_opt = eel_str_strip_substring_and_after (hasmntopt (ent, "dev="),
+									    ",");
+				volume = create_volume (fs_opt+strlen("dev="), ent->mnt_dir);
+				g_free (fs_opt);
+
+			} else {
+#endif
+				volume = create_volume (ent->mnt_fsname, ent->mnt_dir);
+#if defined (HAVE_HASMNTOPT)
+			}
+#endif
 			volumes = finish_creating_volume_and_prepend
 				(monitor, volume, ent->mnt_type, volumes);
 		}
@@ -583,6 +600,21 @@
 	return g_list_sort (g_list_reverse (volumes), (GCompareFunc) floppy_sort);
 }
 
+static gboolean
+entry_is_supermounted_volume (const MountTableEntry *ent, const NautilusVolume *volume)
+{
+      char * fs_opt;
+      gboolean result = FALSE;
+
+      if (strcmp (ent->mnt_type, "supermount") == 0) {
+              fs_opt = eel_str_strip_substring_and_after (hasmntopt (ent, "dev="),
+                                                          ",");
+              result = strcmp (volume->device_path, fs_opt + strlen ("dev=")) == 0;
+              g_free (fs_opt);
+      }
+      return result;
+}
+
 #ifndef SOLARIS_MNT
 
 static gboolean
@@ -617,7 +649,9 @@
 	}
 #elif defined (HAVE_MNTENT_H)
 	while ((ent = getmntent (file)) != NULL) {
-		if (strcmp (volume->device_path, ent->mnt_fsname) == 0
+		if ((strcmp (volume->device_path, ent->mnt_fsname) == 0
+		     || entry_is_supermounted_volume (ent, volume))
+		    && strcmp (volume->mount_path, ent->mnt_dir) == 0
 		    && has_removable_mntent_options (ent)) {
 			removable = TRUE;
 			break;
@@ -772,7 +806,7 @@
 		break;
 
 	default:
-		name = NULL;
+		name = g_strdup (_("CD-ROM"));
   	}
 	
 	close (fd);
@@ -822,7 +856,7 @@
 
 
 static void
-eject_device (char *path)
+eject_device (const char *path)
 {
 	char *command;	
 	
@@ -830,7 +864,6 @@
 		command = g_strdup_printf ("eject %s", path);	
 		eel_gnome_shell_execute (command);
 		g_free (command);
-		g_free (path);
 	}
 }
 
@@ -967,6 +1000,7 @@
 	static time_t last_mtime = 0;
         static FILE *fh = NULL;
         static GList *saved_list = NULL;
+	GList * removable_list;				
         const char *file_name;
 	const char *separator;
 	char line[PATH_MAX * 3];
@@ -1027,6 +1061,19 @@
                         device_path = eel_string_list_nth (list, 0);
                         mount_path = eel_string_list_nth (list, 1);
                         file_system_type_name = eel_string_list_nth (list, 2);
+ 			/* For supermount, search info in removable list */
+ 			if (eel_strcmp ("supermount", file_system_type_name) == 0) {
+ 				removable_list = monitor->details->removable_volumes;
+ 				while (removable_list) {
+ 					volume = (NautilusVolume *) removable_list->data;
+ 					if (eel_strcmp (mount_path, volume->mount_path) == 0) {
+						g_free (device_path);
+ 						device_path = g_strdup (volume->device_path);
+ 						break;
+ 					}
+ 					removable_list = removable_list->next;
+ 				}				
+ 			}
                         volume = create_volume (device_path, mount_path);
 			if (eel_string_list_get_length (list) >= 4 &&
 			    option_list_has_option (eel_string_list_peek_nth (list, 3), MNTOPT_RO))
@@ -1501,10 +1548,13 @@
 		old_locale = g_getenv ("LC_ALL");
 		eel_setenv ("LC_ALL", "C", TRUE);
 
-		open_error_pipe ();
-		file = popen (info->command, "r");
-		close_error_pipe (info->should_mount, info->mount_point);
-		pclose (file);
+		if (info->command != NULL) {
+			open_error_pipe ();
+			file = popen (info->command, "r");
+			close_error_pipe (info->should_mount, info->mount_point);
+			pclose (file);
+			g_free (info->command);
+		}
 
 		if (info->should_eject) {
 			eject_device (info->device_path?info->device_path:info->mount_point);
@@ -1516,7 +1566,6 @@
 			eel_unsetenv("LC_ALL");
 		}
 
-		g_free (info->command);
 		g_free (info->mount_point);
 		g_free (info->device_path);
 		g_free (info);
@@ -1579,7 +1628,11 @@
        }
 
 	mount_info = g_new0 (MountThreadInfo, 1);
-	mount_info->command = g_strdup (command_string);	
+       /* Don't run mount/umount on supermount mount point, it is useless */
+        if ((volume->file_system_type == NULL) 
+	    || (strcmp (volume->file_system_type->name,"supermount") != 0)) {
+		mount_info->command = g_strdup (command_string);	
+	}
 	mount_info->mount_point = g_strdup (mount_point);
 	if (volume) {
 		mount_info->device_path = g_strdup (volume->device_path);