After reviewing the patch further a few issues with
15_uuid_mount.patch presented themselves:

1. The logic to skip non-existent UUID= (and now LABEL=) mounts is
   spurious---it only affects UID= mounts, while leaving intact
   entries with non-existent device nodes, dangling symlinks to device
   nodes, etc.

2. There is a memory leak introduced by 15_uuid_mount.patch: if the
   fstab entry is skipped, mount_entry, mount_entry->device_path,
   and mount_entry->mount_path all need to be freed.

Discussion with J. Mouette on #gnome-debian indicated that it would be
best to keep all the mount points, whether or not the device can be
found.  The attached patch (again, to be applied after
15_uuid_mounts.patch) does that, fixing both of the above problems.


A more long-term solution would involve comparing filesystems by
st_dev rather than device_path; but it is not clear to me how to make
such a change without editing the GnomeVFSUnixMount structure and
potentially breaking binary compatibility.


--- gnome-vfs-2.22.0/libgnomevfs/gnome-vfs-unix-mounts.c	2009-01-15 00:08:55.366081000 -0500
+++ gnome-vfs-2.22.0-new/libgnomevfs/gnome-vfs-unix-mounts.c	2009-01-15 14:52:36.252107709 -0500
@@ -585,7 +585,7 @@
 		
 		mount_entry = g_new0 (GnomeVFSUnixMountPoint, 1);
 
-                /* resolve symlinks */
+                /* resolve symlinks in the mount point */
                 if (realpath (mntent->mnt_dir, rpath))
                     mount_entry->mount_path = g_strdup (rpath);
                 else
@@ -593,28 +593,40 @@
 
 		mount_entry->device_path = g_strdup (mntent->mnt_fsname);
 
+		/* handle UUID= and LABEL= by trying equivalent /dev/disk/ links */
 		if(strlen(mount_entry->device_path) >= 5 && !strncmp (mount_entry->device_path, "UUID=", 5)) {
 			gchar *device_path;
 
 			device_path = g_strdup_printf ("/dev/disk/by-uuid/%s", mount_entry->device_path+5);
-			   
-			if (g_file_test (device_path, G_FILE_TEST_IS_SYMLINK)) {
-				char rpath[PATH_MAX];
-				if (realpath (device_path, rpath)) {
-					g_free (mount_entry->device_path);
-					mount_entry->device_path = g_strdup (rpath);
-				}
-				else {
-					g_free (device_path);
-					continue;
-				}
+			/* if the device does not exist, keep using the old name */
+			if (g_file_test (device_path, G_FILE_TEST_EXISTS)) {
+				g_free (mount_entry->device_path);
+				mount_entry->device_path = device_path;
+			} else {
+				g_free (device_path);
 			}
-			else {
+		}
+		if(strlen(mount_entry->device_path) >= 6 && !strncmp (mount_entry->device_path, "LABEL=", 6)) {
+			gchar *device_path;
+
+			device_path = g_strdup_printf ("/dev/disk/by-label/%s", mount_entry->device_path+6);
+			/* if the device does not exist, keep using the old name */
+			if (g_file_test (device_path, G_FILE_TEST_EXISTS)) {
+				g_free (mount_entry->device_path);
+				mount_entry->device_path = device_path;
+			} else {
 				g_free (device_path);
-				continue;
 			}
-			g_free (device_path);
-		}	
+		}
+
+		/* resolve symlinks in the device */
+		if (g_file_test (mount_entry->device_path, G_FILE_TEST_IS_SYMLINK)) {
+			char rpath[PATH_MAX];
+			if (realpath (mount_entry->device_path, rpath)) {
+				g_free (mount_entry->device_path);
+				mount_entry->device_path = g_strdup(rpath);
+			}
+		}
 
 		mount_entry->filesystem_type = g_strdup (mntent->mnt_type);
 

-- 
Neil Moore, n...@s-z.org, http://s-z.org/neil/

Reply via email to