[PATCH 1/2] mountpoint: return dev_t from dir_to_device

[PATCH 1/2] mountpoint: return dev_t from dir_to_device

[Dave Reisner]

The string returned from this function was never of much use other than
to stat the path when the user requested a major:minor pair beyond the
true/false exit. Save some processing and directly returning the dev_t
on success, and an impossible value on failure.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
 sys-utils/mountpoint.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index f668368..065d96b 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -40,18 +40,18 @@
 
 static int quiet;
 
-static char *dir_to_device(const char *spec)
+static dev_t dir_to_device(const char *spec)
 {
 	struct libmnt_table *tb = mnt_new_table_from_file("/proc/self/mountinfo");
 	struct libmnt_fs *fs;
-	char *res = NULL;
+	dev_t res = (dev_t)-1;
 
 	if (!tb)
-		return NULL;
+		return (dev_t)-1;
 
 	fs = mnt_table_find_target(tb, spec, MNT_ITER_BACKWARD);
 	if (fs && mnt_fs_get_target(fs))
-		res = xstrdup(mnt_fs_get_source(fs));
+		res = mnt_fs_get_devno(fs);
 
 	mnt_free_table(tb);
 	return res;
@@ -146,7 +146,7 @@ int main(int argc, char **argv)
 	if (dev_devno)
 		rc = print_devno(spec, &st);
 	else {
-		char *src;
+		dev_t src;
 
 		if (!S_ISDIR(st.st_mode)) {
 			if (!quiet)
@@ -154,16 +154,15 @@ int main(int argc, char **argv)
 			return EXIT_FAILURE;
 		}
 		src = dir_to_device(spec);
-		if (!src) {
+		if (src == (dev_t)-1) {
 			if (!quiet)
 				printf(_("%s is not a mountpoint\n"), spec);
 			return EXIT_FAILURE;
 		}
 		if (fs_devno)
-			rc = print_devno(src, NULL);
+			printf("%u:%u\n", major(src), minor(src));
 		else if (!quiet)
 			printf(_("%s is a mountpoint\n"), spec);
-		free(src);
 	}
 
 	return rc ? EXIT_FAILURE : EXIT_SUCCESS;
-- 
1.7.7

[PATCH 2/2] mountpoint: fallback on stat when /proc isn't mounted

[Dave Reisner]

mountpoint will fail when /proc isn't available, which lessens its
usefulness as a tool to detect mountpoints. In this case, mimic what
sysvinit's mountpoint tool does and compare the device and inode values
of root and specified target.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
Rationale: I think this patch is especially important for folks who are still
using sysvinit. Particularly in the case of a user not having an initramfs, the
userspace initscripts need to be able to detect, without fail, the presence of
/proc, /sys, and /dev being mounted (and mount them when they aren't). While
checking for /proc first and mounting it when mountpoint returns failure will
result in the correct action, it's reliant on broken behavior -- that is,
mountpoint returned false because /proc isn't mounted, not because /proc isn't
a mountpoint.

 sys-utils/mountpoint.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index 065d96b..1297d82 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -46,8 +46,19 @@ static dev_t dir_to_device(const char *spec)
 	struct libmnt_fs *fs;
 	dev_t res = (dev_t)-1;
 
-	if (!tb)
-		return (dev_t)-1;
+	if (!tb) {
+		struct stat root_st, spec_st;
+
+		/* fallback on using stat when /proc isn't available. return success when
+		 * we're examining different devices or encounter an exact match for the
+		 * root device */
+		if (stat(spec, &spec_st) == 0 && stat("/", &root_st) == 0 &&
+				root_st.st_dev != spec_st.st_dev ||
+				(root_st.st_dev == spec_st.st_dev && root_st.st_ino == spec_st.st_ino))
+			res = spec_st.st_dev;
+
+		return res;
+	}
 
 	fs = mnt_table_find_target(tb, spec, MNT_ITER_BACKWARD);
 	if (fs && mnt_fs_get_target(fs))
-- 
1.7.7

Re: [PATCH 1/2] mountpoint: return dev_t from dir_to_device

[Dave Reisner]

On Sat, Oct 08, 2011 at 11:31:50PM -0400, Dave Reisner wrote:
> The string returned from this function was never of much use other than
> to stat the path when the user requested a major:minor pair beyond the
> true/false exit. Save some processing and directly returning the dev_t
> on success, and an impossible value on failure.
> 
> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> ---
>  sys-utils/mountpoint.c |   15 +++++++--------
>  1 files changed, 7 insertions(+), 8 deletions(-)
> 

I'll also mention that this fixes an issue with mountpoint that causes
it to throw a false negative when /proc is mounted with a source of
"none". This value is treated specially in libmount and sets the source
to NULL, which makes dir_to_device() incorrectly claim that the directory
is not in fact a mountpoint.

Regards,
dave

Re: [PATCH 1/2] mountpoint: return dev_t from dir_to_device

[Karel Zak]

On Sun, Oct 09, 2011 at 05:23:26PM -0400, Dave Reisner wrote:
> On Sat, Oct 08, 2011 at 11:31:50PM -0400, Dave Reisner wrote:
> > The string returned from this function was never of much use other than
> > to stat the path when the user requested a major:minor pair beyond the
> > true/false exit. Save some processing and directly returning the dev_t
> > on success, and an impossible value on failure.
> > 
> > Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> > ---
> >  sys-utils/mountpoint.c |   15 +++++++--------
> >  1 files changed, 7 insertions(+), 8 deletions(-)
> > 
> 
> I'll also mention that this fixes an issue with mountpoint that causes
> it to throw a false negative when /proc is mounted with a source of
> "none". This value is treated specially in libmount and sets the source
> to NULL, which makes dir_to_device() incorrectly claim that the directory
> is not in fact a mountpoint.

 Applied with some minor changes, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/2] mountpoint: fallback on stat when /proc isn't mounted

[Karel Zak]

On Sat, Oct 08, 2011 at 11:31:51PM -0400, Dave Reisner wrote:
> mountpoint will fail when /proc isn't available, which lessens its
> usefulness as a tool to detect mountpoints. In this case, mimic what
> sysvinit's mountpoint tool does and compare the device and inode values
> of root and specified target.
> 
> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> ---
> Rationale: I think this patch is especially important for folks who are still
> using sysvinit. Particularly in the case of a user not having an initramfs, the
> userspace initscripts need to be able to detect, without fail, the presence of
> /proc, /sys, and /dev being mounted (and mount them when they aren't). While
> checking for /proc first and mounting it when mountpoint returns failure will
> result in the correct action, it's reliant on broken behavior -- that is,
> mountpoint returned false because /proc isn't mounted, not because /proc isn't
> a mountpoint.

 Good point.

>  sys-utils/mountpoint.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
> index 065d96b..1297d82 100644
> --- a/sys-utils/mountpoint.c
> +++ b/sys-utils/mountpoint.c
> @@ -46,8 +46,19 @@ static dev_t dir_to_device(const char *spec)
>  	struct libmnt_fs *fs;
>  	dev_t res = (dev_t)-1;
>  
> -	if (!tb)
> -		return (dev_t)-1;
> +	if (!tb) {
> +		struct stat root_st, spec_st;
> +
> +		/* fallback on using stat when /proc isn't available. return success when
> +		 * we're examining different devices or encounter an exact match for the
> +		 * root device */
> +		if (stat(spec, &spec_st) == 0 && stat("/", &root_st) == 0 &&
> +				root_st.st_dev != spec_st.st_dev ||
> +				(root_st.st_dev == spec_st.st_dev && root_st.st_ino == spec_st.st_ino))

 Hmm ... && ... || ..., you forgot brackets here  ;-)

 Anyway, this is very poor solution. The tradition way how to detect
 mountpoint is compare "/path" and /path/.."

 Fixed, applied.
    
        Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com