xfsprogs: libxcmd/paths: make all comparisons using realpath'd paths

PATCH New Eric Sandeen 4 participants 6 messages Jul 12, 2014 – Jul 15, 2014

Share & tools

Eric Sandeen

author

2014-07-12 01:32 UTC #0 lore raw

PATCH 1/1 xfsprogs: libxcmd/paths: make all comparisons using realpath'd paths

Both mountpoints and devices can be symlinks, so given a path to look for, and mountpoints/devices from the system, use realpath() on *everything* before making the comparison to see if our path is a match.

So, with symlinks for mount points as well as for devices:

# ls -l /dev/mapper/testvg-lvol0 
lrwxrwxrwx. 1 root root 7 Jul 11 19:24 /dev/mapper/testvg-lvol0 -> ../dm-3
# ls -l /mnt/scratch2
lrwxrwxrwx. 1 root root 12 Jul 11 19:57 /mnt/scratch2 -> /mnt/scratch

this should all work, and does now:

# xfs_quota -xc "report -h" /mnt/scratch2
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /mnt/scratch
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/dm-3 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/mapper/testvg-lvol0 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

The commit:

050a7f1 xfsprogs: handle symlinks etc in fs_table_initialise_mounts()

tried to fix this earlier, but only worked one way; it compared the argument path in both given and realpath form to the paths in getmntent, but did not compare to the realpaths of the getmntent devices.

If we reduce everything, everywhere, to a realpath(), we've got our best shot at finding the match.

Signed-off-by Eric Sandeen <[email protected]>

---

libxcmd/paths.c +30 ~~-10~~

index 7b0e434..8da3968 100644
@@ -269,6 +269,9 @@ out_nomem:
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are reduced to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntent.
  */
 static int
 fs_table_initialise_mounts(

@@ -278,7 +281,7 @@ fs_table_initialise_mounts(
 	FILE		*mtp;
 	char		*fslog, *fsrt;
 	int		error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmnt_fsname = NULL, *rmnt_dir = NULL;
 
 	error = found = 0;
 	fslog = fsrt = NULL;

@@ -300,11 +303,16 @@ fs_table_initialise_mounts(
 	while ((mnt = getmntent(mtp)) != NULL) {
 		if (strcmp(mnt->mnt_type, "xfs") != 0)
 			continue;
+		free(rmnt_dir);
+		if ((rmnt_dir = realpath(mnt->mnt_dir, NULL)) == NULL)
+			continue;
+		free(rmnt_fsname);
+		if ((rmnt_fsname = realpath(mnt->mnt_fsname, NULL)) == NULL)
+			continue;
+
 		if (path &&
-		    ((strcmp(path, mnt->mnt_dir) != 0) &&
-		     (strcmp(path, mnt->mnt_fsname) != 0) &&
-		     (strcmp(rpath, mnt->mnt_dir) != 0) &&
-		     (strcmp(rpath, mnt->mnt_fsname) != 0)))
+		    ((strcmp(rpath, rmnt_dir) != 0) &&
+		     (strcmp(rpath, rmnt_fsname) != 0)))
 			continue;
 		if (fs_extract_mount_options(mnt, &fslog, &fsrt))
 			continue;

@@ -317,6 +325,8 @@ fs_table_initialise_mounts(
 	}
 	endmntent(mtp);
 	free(rpath);
+	free(rmnt_dir);
+	free(rmnt_fsname);
 
 	if (path && !found)
 		error = ENXIO;

@@ -330,6 +340,9 @@ fs_table_initialise_mounts(
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are reduced to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntinfo.
  */
 static int
 fs_table_initialise_mounts(

@@ -337,7 +350,7 @@ fs_table_initialise_mounts(
 {
 	struct statfs	*stats;
 	int		i, count, error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmntfromname= NULL, *rmntonname= NULL;
 
 	error = found = 0;
 	if ((count = getmntinfo(&stats, 0)) < 0) {

@@ -354,11 +367,16 @@ fs_table_initialise_mounts(
 	for (i = 0; i < count; i++) {
 		if (strcmp(stats[i].f_fstypename, "xfs") != 0)
 			continue;
+		free(rmntfromname);
+		if ((rmntfromname = realpath(stats[i].f_mntfromname, NULL)) == NULL)
+			continue;
+		free(rmntonname);
+		if ((rmntfromname = realpath(stats[i].f_mntonname,  NULL)) == NULL)
+			continue;
+
 		if (path &&
-		    ((strcmp(path, stats[i].f_mntonname) != 0) &&
-		     (strcmp(path, stats[i].f_mntfromname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntonname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntfromname) != 0)))
+		    ((strcmp(rpath, rmntonname) != 0) &&
+		     (strcmp(rpath, rmntfromname) != 0)))
 			continue;
 		/* TODO: external log and realtime device? */
 		(void) fs_table_insert(stats[i].f_mntonname, 0,

@@ -370,6 +388,8 @@ fs_table_initialise_mounts(
 		}
 	}
 	free(rpath);
+	free(rmntfromname);
+	free(rmntonname);
 	if (path && !found)
 		error = ENXIO;

quota/report.c +1 -1

index 70894a2..d486524 100644
@@ -624,7 +624,7 @@ report_f(
 		if (flags & ALL_MOUNTS_FLAG)
 			report_any_type(fp, form, type, NULL,
 					lower, upper, flags);
-		else if (fs_path->fs_flags & FS_MOUNT_POINT)
+		else if (fs_path && fs_path->fs_flags & FS_MOUNT_POINT)
 			report_any_type(fp, form, type, fs_path->fs_dir,
 					lower, upper, flags);
 	} else while (argc > optind) {

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2014-07-12 01:34 UTC #1 lore raw

PATCH v2 1/1 xfsprogs: libxcmd/paths: make all comparisons using realpath'd paths

So, with symlinks for mount points as well as for devices:

# ls -l /dev/mapper/testvg-lvol0 
lrwxrwxrwx. 1 root root 7 Jul 11 19:24 /dev/mapper/testvg-lvol0 -> ../dm-3
# ls -l /mnt/scratch2
lrwxrwxrwx. 1 root root 12 Jul 11 19:57 /mnt/scratch2 -> /mnt/scratch

this should all work, and does now:

# xfs_quota -xc "report -h" /mnt/scratch2
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /mnt/scratch
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/dm-3 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/mapper/testvg-lvol0 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

The commit:

050a7f1 xfsprogs: handle symlinks etc in fs_table_initialise_mounts()

If we reduce everything, everywhere, to a realpath(), we've got our best shot at finding the match.

Signed-off-by Eric Sandeen <[email protected]>

---

V2: remove the quota/report.c change which snuck in...

libxcmd/paths.c +30 ~~-10~~

index 7b0e434..8da3968 100644
@@ -269,6 +269,9 @@ out_nomem:
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are reduced to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntent.
  */
 static int
 fs_table_initialise_mounts(

@@ -278,7 +281,7 @@ fs_table_initialise_mounts(
 	FILE		*mtp;
 	char		*fslog, *fsrt;
 	int		error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmnt_fsname = NULL, *rmnt_dir = NULL;
 
 	error = found = 0;
 	fslog = fsrt = NULL;

@@ -300,11 +303,16 @@ fs_table_initialise_mounts(
 	while ((mnt = getmntent(mtp)) != NULL) {
 		if (strcmp(mnt->mnt_type, "xfs") != 0)
 			continue;
+		free(rmnt_dir);
+		if ((rmnt_dir = realpath(mnt->mnt_dir, NULL)) == NULL)
+			continue;
+		free(rmnt_fsname);
+		if ((rmnt_fsname = realpath(mnt->mnt_fsname, NULL)) == NULL)
+			continue;
+
 		if (path &&
-		    ((strcmp(path, mnt->mnt_dir) != 0) &&
-		     (strcmp(path, mnt->mnt_fsname) != 0) &&
-		     (strcmp(rpath, mnt->mnt_dir) != 0) &&
-		     (strcmp(rpath, mnt->mnt_fsname) != 0)))
+		    ((strcmp(rpath, rmnt_dir) != 0) &&
+		     (strcmp(rpath, rmnt_fsname) != 0)))
 			continue;
 		if (fs_extract_mount_options(mnt, &fslog, &fsrt))
 			continue;

@@ -317,6 +325,8 @@ fs_table_initialise_mounts(
 	}
 	endmntent(mtp);
 	free(rpath);
+	free(rmnt_dir);
+	free(rmnt_fsname);
 
 	if (path && !found)
 		error = ENXIO;

@@ -330,6 +340,9 @@ fs_table_initialise_mounts(
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are reduced to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntinfo.
  */
 static int
 fs_table_initialise_mounts(

@@ -337,7 +350,7 @@ fs_table_initialise_mounts(
 {
 	struct statfs	*stats;
 	int		i, count, error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmntfromname= NULL, *rmntonname= NULL;
 
 	error = found = 0;
 	if ((count = getmntinfo(&stats, 0)) < 0) {

@@ -354,11 +367,16 @@ fs_table_initialise_mounts(
 	for (i = 0; i < count; i++) {
 		if (strcmp(stats[i].f_fstypename, "xfs") != 0)
 			continue;
+		free(rmntfromname);
+		if ((rmntfromname = realpath(stats[i].f_mntfromname, NULL)) == NULL)
+			continue;
+		free(rmntonname);
+		if ((rmntfromname = realpath(stats[i].f_mntonname,  NULL)) == NULL)
+			continue;
+
 		if (path &&
-		    ((strcmp(path, stats[i].f_mntonname) != 0) &&
-		     (strcmp(path, stats[i].f_mntfromname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntonname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntfromname) != 0)))
+		    ((strcmp(rpath, rmntonname) != 0) &&
+		     (strcmp(rpath, rmntfromname) != 0)))
 			continue;
 		/* TODO: external log and realtime device? */
 		(void) fs_table_insert(stats[i].f_mntonname, 0,

@@ -370,6 +388,8 @@ fs_table_initialise_mounts(
 		}
 	}
 	free(rpath);
+	free(rmntfromname);
+	free(rmntonname);
 	if (path && !found)
 		error = ENXIO;

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Brian Foster

2014-07-14 12:34 UTC #2 lore raw

Quoted 123 lines · Eric Sandeen

So, with symlinks for mount points as well as for devices:

# ls -l /dev/mapper/testvg-lvol0 
lrwxrwxrwx. 1 root root 7 Jul 11 19:24 /dev/mapper/testvg-lvol0 -> ../dm-3
# ls -l /mnt/scratch2
lrwxrwxrwx. 1 root root 12 Jul 11 19:57 /mnt/scratch2 -> /mnt/scratch

this should all work, and does now:

# xfs_quota -xc "report -h" /mnt/scratch2
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /mnt/scratch
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/dm-3 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/mapper/testvg-lvol0 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

The commit:

050a7f1 xfsprogs: handle symlinks etc in fs_table_initialise_mounts()

If we reduce everything, everywhere, to a realpath(), we've got our best shot at finding the match.

Signed-off-by Eric Sandeen <[email protected]>

---

V2: remove the quota/report.c change which snuck in...

libxcmd/paths.c +18 -5

index 7b0e434..8da3968 100644
@@ -269,6 +269,9 @@ out_nomem:
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are reduced to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntent.
  */
 static int
 fs_table_initialise_mounts(

@@ -278,7 +281,7 @@ fs_table_initialise_mounts(
 	FILE		*mtp;
 	char		*fslog, *fsrt;
 	int		error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmnt_fsname = NULL, *rmnt_dir = NULL;
 
 	error = found = 0;
 	fslog = fsrt = NULL;

@@ -300,11 +303,16 @@ fs_table_initialise_mounts(
 	while ((mnt = getmntent(mtp)) != NULL) {
 		if (strcmp(mnt->mnt_type, "xfs") != 0)
 			continue;
+		free(rmnt_dir);
+		if ((rmnt_dir = realpath(mnt->mnt_dir, NULL)) == NULL)
+			continue;
+		free(rmnt_fsname);
+		if ((rmnt_fsname = realpath(mnt->mnt_fsname, NULL)) == NULL)
+			continue;
+
 		if (path &&
-		    ((strcmp(path, mnt->mnt_dir) != 0) &&
-		     (strcmp(path, mnt->mnt_fsname) != 0) &&
-		     (strcmp(rpath, mnt->mnt_dir) != 0) &&
-		     (strcmp(rpath, mnt->mnt_fsname) != 0)))
+		    ((strcmp(rpath, rmnt_dir) != 0) &&
+		     (strcmp(rpath, rmnt_fsname) != 0)))
 			continue;
 		if (fs_extract_mount_options(mnt, &fslog, &fsrt))
 			continue;

@@ -317,6 +325,8 @@ fs_table_initialise_mounts(
 	}
 	endmntent(mtp);
 	free(rpath);
+	free(rmnt_dir);
+	free(rmnt_fsname);
 
 	if (path && !found)
 		error = ENXIO;

@@ -330,6 +340,9 @@ fs_table_initialise_mounts(
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are reduced to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntinfo.
  */
 static int
 fs_table_initialise_mounts(

libxcmd/paths.c fs_table_initialise_mounts( · line 353

Quoted 5 lines

libxcmd/paths.c +1 -1

@@ -337,7 +350,7 @@ fs_table_initialise_mounts(
 {
 	struct statfs	*stats;
 	int		i, count, error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmntfromname= NULL, *rmntonname= NULL;

A couple missing spaces before '=' here.

The fundamental change looks good, but the memory allocation handling seems a little ugly to me. A 'next:' label in the loop that frees the path buffers is cleaner IMO. Another option could be to put a couple PATH_MAX buffers on the stack or allocate them directly to also eliminate the realpath() return value assignment..?

Brian

Quoted 34 lines

libxcmd/paths.c +11 -4

 	error = found = 0;
 	if ((count = getmntinfo(&stats, 0)) < 0) {

@@ -354,11 +367,16 @@ fs_table_initialise_mounts(
 	for (i = 0; i < count; i++) {
 		if (strcmp(stats[i].f_fstypename, "xfs") != 0)
 			continue;
+		free(rmntfromname);
+		if ((rmntfromname = realpath(stats[i].f_mntfromname, NULL)) == NULL)
+			continue;
+		free(rmntonname);
+		if ((rmntfromname = realpath(stats[i].f_mntonname,  NULL)) == NULL)
+			continue;
+
 		if (path &&
-		    ((strcmp(path, stats[i].f_mntonname) != 0) &&
-		     (strcmp(path, stats[i].f_mntfromname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntonname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntfromname) != 0)))
+		    ((strcmp(rpath, rmntonname) != 0) &&
+		     (strcmp(rpath, rmntfromname) != 0)))
 			continue;
 		/* TODO: external log and realtime device? */
 		(void) fs_table_insert(stats[i].f_mntonname, 0,

@@ -370,6 +388,8 @@ fs_table_initialise_mounts(
 		}
 	}
 	free(rpath);
+	free(rmntfromname);
+	free(rmntonname);
 	if (path && !found)
 		error = ENXIO;

Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2014-07-14 14:11 UTC #3 lore raw

Quoted 7 lines · Brian Foster

Quoted 6 lines · Eric Sandeen
Both mountpoints and devices can be symlinks, so given a path to look for, and mountpoints/devices from the system, use realpath() on *everything* before making the comparison to see if our path is a match.
So, with symlinks for mount points as well as for devices:

<snip>

Quoted 8 lines

libxcmd/paths.c fs_table_initialise_mounts( · line 353
Quoted 6 lines
libxcmd/paths.c +1 -1
@@ -337,7 +350,7 @@ fs_table_initialise_mounts(
 {
 	struct statfs	*stats;
 	int		i, count, error, found;
-	char		*rpath = NULL;
+	char		*rpath = NULL, *rmntfromname= NULL, *rmntonname= NULL;
A couple missing spaces before '=' here.

whoopsies

Quoted 5 lines

The fundamental change looks good, but the memory allocation handling seems a little ugly to me. A 'next:' label in the loop that frees the path buffers is cleaner IMO. Another option could be to put a couple PATH_MAX buffers on the stack or allocate them directly to also eliminate the realpath() return value assignment..?

Yeah, that probably makes more sense, thanks.

-eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2014-07-14 21:21 UTC #4 lore raw

PATCH v3 1/1 xfsprogs: libxcmd/paths: make all comparisons using realpath'd paths

In reply to Eric Sandeen

So, with symlinks for mount points as well as for devices:

# ls -l /dev/mapper/testvg-lvol0 
lrwxrwxrwx. 1 root root 7 Jul 11 19:24 /dev/mapper/testvg-lvol0 -> ../dm-3
# ls -l /mnt/scratch2
lrwxrwxrwx. 1 root root 12 Jul 11 19:57 /mnt/scratch2 -> /mnt/scratch

this should all work, and does now:

# xfs_quota -xc "report -h" /mnt/scratch2
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /mnt/scratch
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/dm-3 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

# xfs_quota -xc "report -h" /dev/mapper/testvg-lvol0 
User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
                        Blocks              
User ID      Used   Soft   Hard Warn/Grace   
---------- --------------------------------- 
root            0      0      0  00 [------]

The commit:

050a7f1 xfsprogs: handle symlinks etc in fs_table_initialise_mounts()

If we reduce everything, everywhere, to a realpath(), we've got our best shot at finding the match.

Signed-off-by Eric Sandeen <[email protected]>

---

V2: remove the quota/report.c change which snuck in...
V3: don't use dynamically allocated realpaths

libxcmd/paths.c +26 ~~-16~~

index 7b0e434..443adbb 100644
@@ -269,6 +269,9 @@ out_nomem:
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are boiled down to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntent.
  */
 static int
 fs_table_initialise_mounts(

@@ -278,7 +281,7 @@ fs_table_initialise_mounts(
 	FILE		*mtp;
 	char		*fslog, *fsrt;
 	int		error, found;
-	char		*rpath = NULL;
+	char		rpath[PATH_MAX], rmnt_fsname[PATH_MAX], rmnt_dir[PATH_MAX];
 
 	error = found = 0;
 	fslog = fsrt = NULL;

@@ -294,17 +297,20 @@ fs_table_initialise_mounts(
 
 	/* Use realpath to resolve symlinks, relative paths, etc */
 	if (path)
-		if ((rpath = realpath(path, NULL)) == NULL)
-			return ENOENT;
+		if (!realpath(path, rpath))
+			return errno;
 
 	while ((mnt = getmntent(mtp)) != NULL) {
 		if (strcmp(mnt->mnt_type, "xfs") != 0)
 			continue;
+		if (!realpath(mnt->mnt_dir, rmnt_dir))
+			continue;
+		if (!realpath(mnt->mnt_fsname, rmnt_fsname))
+			continue;
+
 		if (path &&
-		    ((strcmp(path, mnt->mnt_dir) != 0) &&
-		     (strcmp(path, mnt->mnt_fsname) != 0) &&
-		     (strcmp(rpath, mnt->mnt_dir) != 0) &&
-		     (strcmp(rpath, mnt->mnt_fsname) != 0)))
+		    ((strcmp(rpath, rmnt_dir) != 0) &&
+		     (strcmp(rpath, rmnt_fsname) != 0)))
 			continue;
 		if (fs_extract_mount_options(mnt, &fslog, &fsrt))
 			continue;

@@ -316,7 +322,6 @@ fs_table_initialise_mounts(
 		}
 	}
 	endmntent(mtp);
-	free(rpath);
 
 	if (path && !found)
 		error = ENXIO;

@@ -330,6 +335,9 @@ fs_table_initialise_mounts(
 /*
  * If *path is NULL, initialize the fs table with all xfs mount points in mtab
  * If *path is specified, search for that path in mtab
+ *
+ * Everything - path, devices, and mountpoints - are boiled down to realpath()
+ * for comparison, but fs_table is populated with what comes from getmntinfo.
  */
 static int
 fs_table_initialise_mounts(

@@ -337,7 +345,7 @@ fs_table_initialise_mounts(
 {
 	struct statfs	*stats;
 	int		i, count, error, found;
-	char		*rpath = NULL;
+	char		rpath[PATH_MAX], rmntfromname[PATH_MAX], rmntonname[PATH_MAX];
 
 	error = found = 0;
 	if ((count = getmntinfo(&stats, 0)) < 0) {

@@ -348,17 +356,20 @@ fs_table_initialise_mounts(
 
 	/* Use realpath to resolve symlinks, relative paths, etc */
 	if (path)
-		if ((rpath = realpath(path, NULL)) == NULL)
-			return ENOENT;
+		if (!realpath(path, rpath))
+			return errno;
 
 	for (i = 0; i < count; i++) {
 		if (strcmp(stats[i].f_fstypename, "xfs") != 0)
 			continue;
+		if (!realpath(stats[i].f_mntfromname, rmntfromname))
+			continue;
+		if (!realpath(stats[i].f_mntonname, rmnttomname)))
+			continue;
+
 		if (path &&
-		    ((strcmp(path, stats[i].f_mntonname) != 0) &&
-		     (strcmp(path, stats[i].f_mntfromname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntonname) != 0) &&
-		     (strcmp(rpath, stats[i].f_mntfromname) != 0)))
+		    ((strcmp(rpath, rmntonname) != 0) &&
+		     (strcmp(rpath, rmntfromname) != 0)))
 			continue;
 		/* TODO: external log and realtime device? */
 		(void) fs_table_insert(stats[i].f_mntonname, 0,

@@ -369,7 +380,6 @@ fs_table_initialise_mounts(
 			break;
 		}
 	}
-	free(rpath);
 	if (path && !found)
 		error = ENXIO;

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

Reviewed-by on v3 1/1

2014-07-15 10:13 UTC #5 lore raw

Looks good,

Reviewed-by Christoph Hellwig <[email protected]>

Can you also create a testcase for it? Should be simple enough by creating symlinks for the scratch device and mount and testing the quota tool on it.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs