[PATCH 1/3] findmnt: reference correct manual section

[PATCH 1/3] findmnt: reference correct manual section

[Dave Reisner]

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
 misc-utils/findmnt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/misc-utils/findmnt.c b/misc-utils/findmnt.c
index 1d34696..9005483 100644
--- a/misc-utils/findmnt.c
+++ b/misc-utils/findmnt.c
@@ -1005,7 +1005,7 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
 	for (i = 0; i < FINDMNT_NCOLUMNS; i++)
 		fprintf(out, " %11s  %s\n", infos[i].name, _(infos[i].help));
 
-	fprintf(out, USAGE_MAN_TAIL("findmnt(1)"));
+	fprintf(out, USAGE_MAN_TAIL("findmnt(8)"));
 
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
-- 
1.7.10.2

[PATCH 2/3] findmnt: fallback to mountinfo for polling

[Dave Reisner]

If no tabfiles are specified, default to watching /proc/self/mountinfo
rather than exiting silently with EXIT_FAILURE.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
 misc-utils/findmnt.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/misc-utils/findmnt.c b/misc-utils/findmnt.c
index 9005483..eab224a 100644
--- a/misc-utils/findmnt.c
+++ b/misc-utils/findmnt.c
@@ -1320,8 +1320,7 @@ int main(int argc, char *argv[])
 	 */
 	if (flags & FL_POLL) {
 		/* poll mode (accept the first tabfile only) */
-		if (tabfiles && ntabfiles > 0)
-			rc = poll_table(tb, *tabfiles, timeout, tt, direction);
+		rc = poll_table(tb, tabfiles ? *tabfiles : _PATH_PROC_MOUNTINFO, timeout, tt, direction);
 
 	} else if ((tt_flags & TT_FL_TREE) && is_listall_mode())
 		/* whole tree */
-- 
1.7.10.2

[PATCH 3/3] libmount/utils: Use binary search to compare pseudofs

[Dave Reisner]

While we're at it, expand this list of known pseudofs types.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
 libmount/src/utils.c |   68 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/libmount/src/utils.c b/libmount/src/utils.c
index b824edc..77557f2 100644
--- a/libmount/src/utils.c
+++ b/libmount/src/utils.c
@@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
 	return rc;
 }
 
+/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
+static int fstype_cmp(const void *v1, const void *v2)
+{
+	const char *s1 = *(const char **)v1;
+	const char *s2 = *(const char **)v2;
+
+	return strcmp(s1, s2);
+}
+
 /* returns basename and keeps dirname in the @path, if @path is "/" (root)
  * then returns empty string */
 static char *stripoff_last_component(char *path)
@@ -214,31 +223,40 @@ char *mnt_unmangle(const char *str)
  */
 int mnt_fstype_is_pseudofs(const char *type)
 {
-	if (!type)
-		return 0;
-	if (strcmp(type, "none")  == 0 ||
-	    strcmp(type, "proc")  == 0 ||
-	    strcmp(type, "tmpfs") == 0 ||
-	    strcmp(type, "sysfs") == 0 ||
-	    strcmp(type, "autofs") == 0 ||
-	    strcmp(type, "devpts") == 0||
-	    strcmp(type, "cgroup") == 0 ||
-	    strcmp(type, "devtmpfs") == 0 ||
-	    strcmp(type, "devfs") == 0 ||
-	    strcmp(type, "dlmfs") == 0 ||
-	    strcmp(type, "cpuset") == 0 ||
-	    strcmp(type, "configfs") == 0 ||
-	    strcmp(type, "securityfs") == 0 ||
-	    strcmp(type, "hugetlbfs") == 0 ||
-	    strcmp(type, "rpc_pipefs") == 0 ||
-	    strcmp(type, "fusectl") == 0 ||
-	    strcmp(type, "mqueue") == 0 ||
-	    strcmp(type, "binfmt_misc") == 0 ||
-	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
-	    strcmp(type, "debugfs") == 0 ||
-	    strcmp(type, "spufs") == 0)
-		return 1;
-	return 0;
+	static const char *pseudofs[] = {
+		"anon_inodefs",
+		"autofs",
+		"bdev",
+		"binfmt_misc",
+		"cgroup",
+		"configfs",
+		"cpuset",
+		"debugfs",
+		"devfs",
+		"devpts",
+		"devtmpfs",
+		"dlmfs",
+		"fuse.gvfs-fuse-daemon",
+		"fusectl",
+		"hugetlbfs",
+		"mqueue",
+		"nfsd",
+		"none",
+		"pipefs",
+		"proc",
+		"pstore",
+		"ramfs",
+		"rootfs",
+		"rpc_pipefs",
+		"securityfs",
+		"sockfs",
+		"spufs",
+		"sysfs",
+		"tmpfs"
+	};
+
+	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
+				sizeof(char*), fstype_cmp) == NULL);
 }
 
 /**
-- 
1.7.10.2

Re: [PATCH 3/3] libmount/utils: Use binary search to compare pseudofs

[Davidlohr Bueso]

On Sat, 2012-05-19 at 20:39 -0400, Dave Reisner wrote:
> While we're at it, expand this list of known pseudofs types.
> 
> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> ---
>  libmount/src/utils.c |   68 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 43 insertions(+), 25 deletions(-)
> 
> diff --git a/libmount/src/utils.c b/libmount/src/utils.c
> index b824edc..77557f2 100644
> --- a/libmount/src/utils.c
> +++ b/libmount/src/utils.c
> @@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
>  	return rc;
>  }
>  
> +/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
> +static int fstype_cmp(const void *v1, const void *v2)
> +{
> +	const char *s1 = *(const char **)v1;
> +	const char *s2 = *(const char **)v2;
> +
> +	return strcmp(s1, s2);
> +}
> +
>  /* returns basename and keeps dirname in the @path, if @path is "/" (root)
>   * then returns empty string */
>  static char *stripoff_last_component(char *path)
> @@ -214,31 +223,40 @@ char *mnt_unmangle(const char *str)
>   */
>  int mnt_fstype_is_pseudofs(const char *type)
>  {
> -	if (!type)
> -		return 0;
> -	if (strcmp(type, "none")  == 0 ||
> -	    strcmp(type, "proc")  == 0 ||
> -	    strcmp(type, "tmpfs") == 0 ||
> -	    strcmp(type, "sysfs") == 0 ||
> -	    strcmp(type, "autofs") == 0 ||
> -	    strcmp(type, "devpts") == 0||
> -	    strcmp(type, "cgroup") == 0 ||
> -	    strcmp(type, "devtmpfs") == 0 ||
> -	    strcmp(type, "devfs") == 0 ||
> -	    strcmp(type, "dlmfs") == 0 ||
> -	    strcmp(type, "cpuset") == 0 ||
> -	    strcmp(type, "configfs") == 0 ||
> -	    strcmp(type, "securityfs") == 0 ||
> -	    strcmp(type, "hugetlbfs") == 0 ||
> -	    strcmp(type, "rpc_pipefs") == 0 ||
> -	    strcmp(type, "fusectl") == 0 ||
> -	    strcmp(type, "mqueue") == 0 ||
> -	    strcmp(type, "binfmt_misc") == 0 ||
> -	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
> -	    strcmp(type, "debugfs") == 0 ||
> -	    strcmp(type, "spufs") == 0)
> -		return 1;
> -	return 0;
> +	static const char *pseudofs[] = {
> +		"anon_inodefs",
> +		"autofs",
> +		"bdev",
> +		"binfmt_misc",
> +		"cgroup",
> +		"configfs",
> +		"cpuset",
> +		"debugfs",
> +		"devfs",
> +		"devpts",
> +		"devtmpfs",
> +		"dlmfs",
> +		"fuse.gvfs-fuse-daemon",
> +		"fusectl",
> +		"hugetlbfs",
> +		"mqueue",
> +		"nfsd",
> +		"none",
> +		"pipefs",
> +		"proc",
> +		"pstore",
> +		"ramfs",
> +		"rootfs",
> +		"rpc_pipefs",
> +		"securityfs",
> +		"sockfs",
> +		"spufs",
> +		"sysfs",
> +		"tmpfs"
> +	};
> +

What about applying sort (qsort(3)) before searching here? This would
correct any manual string sorting that pseudofs[] imposes to the
developer.

> +	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
> +				sizeof(char*), fstype_cmp) == NULL);
>  }
>  
>  /**

Re: [PATCH 3/3] libmount/utils: Use binary search to compare pseudofs

[Dave Reisner]

On Sun, May 20, 2012 at 04:06:00PM +0200, Davidlohr Bueso wrote:
> On Sat, 2012-05-19 at 20:39 -0400, Dave Reisner wrote:
> > While we're at it, expand this list of known pseudofs types.
> > 
> > Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> > ---
> >  libmount/src/utils.c |   68 +++++++++++++++++++++++++++++++-------------------
> >  1 file changed, 43 insertions(+), 25 deletions(-)
> > 
> > diff --git a/libmount/src/utils.c b/libmount/src/utils.c
> > index b824edc..77557f2 100644
> > --- a/libmount/src/utils.c
> > +++ b/libmount/src/utils.c
> > @@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
> >  	return rc;
> >  }
> >  
> > +/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
> > +static int fstype_cmp(const void *v1, const void *v2)
> > +{
> > +	const char *s1 = *(const char **)v1;
> > +	const char *s2 = *(const char **)v2;
> > +
> > +	return strcmp(s1, s2);
> > +}
> > +
> >  /* returns basename and keeps dirname in the @path, if @path is "/" (root)
> >   * then returns empty string */
> >  static char *stripoff_last_component(char *path)
> > @@ -214,31 +223,40 @@ char *mnt_unmangle(const char *str)
> >   */
> >  int mnt_fstype_is_pseudofs(const char *type)
> >  {
> > -	if (!type)
> > -		return 0;
> > -	if (strcmp(type, "none")  == 0 ||
> > -	    strcmp(type, "proc")  == 0 ||
> > -	    strcmp(type, "tmpfs") == 0 ||
> > -	    strcmp(type, "sysfs") == 0 ||
> > -	    strcmp(type, "autofs") == 0 ||
> > -	    strcmp(type, "devpts") == 0||
> > -	    strcmp(type, "cgroup") == 0 ||
> > -	    strcmp(type, "devtmpfs") == 0 ||
> > -	    strcmp(type, "devfs") == 0 ||
> > -	    strcmp(type, "dlmfs") == 0 ||
> > -	    strcmp(type, "cpuset") == 0 ||
> > -	    strcmp(type, "configfs") == 0 ||
> > -	    strcmp(type, "securityfs") == 0 ||
> > -	    strcmp(type, "hugetlbfs") == 0 ||
> > -	    strcmp(type, "rpc_pipefs") == 0 ||
> > -	    strcmp(type, "fusectl") == 0 ||
> > -	    strcmp(type, "mqueue") == 0 ||
> > -	    strcmp(type, "binfmt_misc") == 0 ||
> > -	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
> > -	    strcmp(type, "debugfs") == 0 ||
> > -	    strcmp(type, "spufs") == 0)
> > -		return 1;
> > -	return 0;
> > +	static const char *pseudofs[] = {
> > +		"anon_inodefs",
> > +		"autofs",
> > +		"bdev",
> > +		"binfmt_misc",
> > +		"cgroup",
> > +		"configfs",
> > +		"cpuset",
> > +		"debugfs",
> > +		"devfs",
> > +		"devpts",
> > +		"devtmpfs",
> > +		"dlmfs",
> > +		"fuse.gvfs-fuse-daemon",
> > +		"fusectl",
> > +		"hugetlbfs",
> > +		"mqueue",
> > +		"nfsd",
> > +		"none",
> > +		"pipefs",
> > +		"proc",
> > +		"pstore",
> > +		"ramfs",
> > +		"rootfs",
> > +		"rpc_pipefs",
> > +		"securityfs",
> > +		"sockfs",
> > +		"spufs",
> > +		"sysfs",
> > +		"tmpfs"
> > +	};
> > +
> 
> What about applying sort (qsort(3)) before searching here? This would
> correct any manual string sorting that pseudofs[] imposes to the
> developer.
> 

Hmm, so the overhead from this seems to be negligible, but given that
this is running as part of a library, wouldn't we need a reentrant
version of qsort? qsort_r was added to glibc in 2.8, but I'm not
familiar with how portable this would be to other libc's.

> > +	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
> > +				sizeof(char*), fstype_cmp) == NULL);
> >  }
> >  
> >  /**
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] libmount/utils: Use binary search to compare pseudofs

[Davidlohr Bueso]

On Sun, 2012-05-20 at 10:51 -0400, Dave Reisner wrote:
> On Sun, May 20, 2012 at 04:06:00PM +0200, Davidlohr Bueso wrote:
> > On Sat, 2012-05-19 at 20:39 -0400, Dave Reisner wrote:
> > > While we're at it, expand this list of known pseudofs types.
> > > 
> > > Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> > > ---
> > >  libmount/src/utils.c |   68 +++++++++++++++++++++++++++++++-------------------
> > >  1 file changed, 43 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/libmount/src/utils.c b/libmount/src/utils.c
> > > index b824edc..77557f2 100644
> > > --- a/libmount/src/utils.c
> > > +++ b/libmount/src/utils.c
> > > @@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
> > >  	return rc;
> > >  }
> > >  
> > > +/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
> > > +static int fstype_cmp(const void *v1, const void *v2)
> > > +{
> > > +	const char *s1 = *(const char **)v1;
> > > +	const char *s2 = *(const char **)v2;
> > > +
> > > +	return strcmp(s1, s2);
> > > +}
> > > +
> > >  /* returns basename and keeps dirname in the @path, if @path is "/" (root)
> > >   * then returns empty string */
> > >  static char *stripoff_last_component(char *path)
> > > @@ -214,31 +223,40 @@ char *mnt_unmangle(const char *str)
> > >   */
> > >  int mnt_fstype_is_pseudofs(const char *type)
> > >  {
> > > -	if (!type)
> > > -		return 0;
> > > -	if (strcmp(type, "none")  == 0 ||
> > > -	    strcmp(type, "proc")  == 0 ||
> > > -	    strcmp(type, "tmpfs") == 0 ||
> > > -	    strcmp(type, "sysfs") == 0 ||
> > > -	    strcmp(type, "autofs") == 0 ||
> > > -	    strcmp(type, "devpts") == 0||
> > > -	    strcmp(type, "cgroup") == 0 ||
> > > -	    strcmp(type, "devtmpfs") == 0 ||
> > > -	    strcmp(type, "devfs") == 0 ||
> > > -	    strcmp(type, "dlmfs") == 0 ||
> > > -	    strcmp(type, "cpuset") == 0 ||
> > > -	    strcmp(type, "configfs") == 0 ||
> > > -	    strcmp(type, "securityfs") == 0 ||
> > > -	    strcmp(type, "hugetlbfs") == 0 ||
> > > -	    strcmp(type, "rpc_pipefs") == 0 ||
> > > -	    strcmp(type, "fusectl") == 0 ||
> > > -	    strcmp(type, "mqueue") == 0 ||
> > > -	    strcmp(type, "binfmt_misc") == 0 ||
> > > -	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
> > > -	    strcmp(type, "debugfs") == 0 ||
> > > -	    strcmp(type, "spufs") == 0)
> > > -		return 1;
> > > -	return 0;
> > > +	static const char *pseudofs[] = {
> > > +		"anon_inodefs",
> > > +		"autofs",
> > > +		"bdev",
> > > +		"binfmt_misc",
> > > +		"cgroup",
> > > +		"configfs",
> > > +		"cpuset",
> > > +		"debugfs",
> > > +		"devfs",
> > > +		"devpts",
> > > +		"devtmpfs",
> > > +		"dlmfs",
> > > +		"fuse.gvfs-fuse-daemon",
> > > +		"fusectl",
> > > +		"hugetlbfs",
> > > +		"mqueue",
> > > +		"nfsd",
> > > +		"none",
> > > +		"pipefs",
> > > +		"proc",
> > > +		"pstore",
> > > +		"ramfs",
> > > +		"rootfs",
> > > +		"rpc_pipefs",
> > > +		"securityfs",
> > > +		"sockfs",
> > > +		"spufs",
> > > +		"sysfs",
> > > +		"tmpfs"
> > > +	};
> > > +
> > 
> > What about applying sort (qsort(3)) before searching here? This would
> > correct any manual string sorting that pseudofs[] imposes to the
> > developer.
> > 
> 
> Hmm, so the overhead from this seems to be negligible, but given that
> this is running as part of a library, wouldn't we need a reentrant
> version of qsort? qsort_r was added to glibc in 2.8, but I'm not
> familiar with how portable this would be to other libc's.

I normally worry about reentrancy when dealing with interrupt handling,
like signals. Also consider that pseudofs[] isn't global.

> 
> > > +	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
> > > +				sizeof(char*), fstype_cmp) == NULL);
> > >  }
> > >  
> > >  /**
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe util-linux" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] libmount/utils: Use binary search to compare pseudofs

[Dave Reisner]

On Sun, May 20, 2012 at 06:05:12PM +0200, Davidlohr Bueso wrote:
> On Sun, 2012-05-20 at 10:51 -0400, Dave Reisner wrote:
> > On Sun, May 20, 2012 at 04:06:00PM +0200, Davidlohr Bueso wrote:
> > > On Sat, 2012-05-19 at 20:39 -0400, Dave Reisner wrote:
> > > > While we're at it, expand this list of known pseudofs types.
> > > > 
> > > > Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> > > > ---
> > > >  libmount/src/utils.c |   68 +++++++++++++++++++++++++++++++-------------------
> > > >  1 file changed, 43 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/libmount/src/utils.c b/libmount/src/utils.c
> > > > index b824edc..77557f2 100644
> > > > --- a/libmount/src/utils.c
> > > > +++ b/libmount/src/utils.c
> > > > @@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
> > > >  	return rc;
> > > >  }
> > > >  
> > > > +/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
> > > > +static int fstype_cmp(const void *v1, const void *v2)
> > > > +{
> > > > +	const char *s1 = *(const char **)v1;
> > > > +	const char *s2 = *(const char **)v2;
> > > > +
> > > > +	return strcmp(s1, s2);
> > > > +}
> > > > +
> > > >  /* returns basename and keeps dirname in the @path, if @path is "/" (root)
> > > >   * then returns empty string */
> > > >  static char *stripoff_last_component(char *path)
> > > > @@ -214,31 +223,40 @@ char *mnt_unmangle(const char *str)
> > > >   */
> > > >  int mnt_fstype_is_pseudofs(const char *type)
> > > >  {
> > > > -	if (!type)
> > > > -		return 0;
> > > > -	if (strcmp(type, "none")  == 0 ||
> > > > -	    strcmp(type, "proc")  == 0 ||
> > > > -	    strcmp(type, "tmpfs") == 0 ||
> > > > -	    strcmp(type, "sysfs") == 0 ||
> > > > -	    strcmp(type, "autofs") == 0 ||
> > > > -	    strcmp(type, "devpts") == 0||
> > > > -	    strcmp(type, "cgroup") == 0 ||
> > > > -	    strcmp(type, "devtmpfs") == 0 ||
> > > > -	    strcmp(type, "devfs") == 0 ||
> > > > -	    strcmp(type, "dlmfs") == 0 ||
> > > > -	    strcmp(type, "cpuset") == 0 ||
> > > > -	    strcmp(type, "configfs") == 0 ||
> > > > -	    strcmp(type, "securityfs") == 0 ||
> > > > -	    strcmp(type, "hugetlbfs") == 0 ||
> > > > -	    strcmp(type, "rpc_pipefs") == 0 ||
> > > > -	    strcmp(type, "fusectl") == 0 ||
> > > > -	    strcmp(type, "mqueue") == 0 ||
> > > > -	    strcmp(type, "binfmt_misc") == 0 ||
> > > > -	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
> > > > -	    strcmp(type, "debugfs") == 0 ||
> > > > -	    strcmp(type, "spufs") == 0)
> > > > -		return 1;
> > > > -	return 0;
> > > > +	static const char *pseudofs[] = {
> > > > +		"anon_inodefs",
> > > > +		"autofs",
> > > > +		"bdev",
> > > > +		"binfmt_misc",
> > > > +		"cgroup",
> > > > +		"configfs",
> > > > +		"cpuset",
> > > > +		"debugfs",
> > > > +		"devfs",
> > > > +		"devpts",
> > > > +		"devtmpfs",
> > > > +		"dlmfs",
> > > > +		"fuse.gvfs-fuse-daemon",
> > > > +		"fusectl",
> > > > +		"hugetlbfs",
> > > > +		"mqueue",
> > > > +		"nfsd",
> > > > +		"none",
> > > > +		"pipefs",
> > > > +		"proc",
> > > > +		"pstore",
> > > > +		"ramfs",
> > > > +		"rootfs",
> > > > +		"rpc_pipefs",
> > > > +		"securityfs",
> > > > +		"sockfs",
> > > > +		"spufs",
> > > > +		"sysfs",
> > > > +		"tmpfs"
> > > > +	};
> > > > +
> > > 
> > > What about applying sort (qsort(3)) before searching here? This would
> > > correct any manual string sorting that pseudofs[] imposes to the
> > > developer.
> > > 
> > 
> > Hmm, so the overhead from this seems to be negligible, but given that
> > this is running as part of a library, wouldn't we need a reentrant
> > version of qsort? qsort_r was added to glibc in 2.8, but I'm not
> > familiar with how portable this would be to other libc's.
> 
> I normally worry about reentrancy when dealing with interrupt handling,
> like signals. Also consider that pseudofs[] isn't global.
> 

qsort_r takes an extra param to the callback to hold an element while
its being swapped. The reentrancy of the function isn't guaranteed just
because it operates on a non-global var.

Regardless, I'm not really in favor of adding this. I think it's reasonable
to push a little responsibility on the developer and preemptively ensure
that this is sorted. Perhaps add a comment in the function would be
sufficient: "make sure this is sorted or Karel will come after you".

d

> > 
> > > > +	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
> > > > +				sizeof(char*), fstype_cmp) == NULL);
> > > >  }
> > > >  
> > > >  /**
> > > 
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe util-linux" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 

Re: [PATCH 3/3] libmount/utils: Use binary search to compare pseudofs

[Davidlohr Bueso]

On Sun, 2012-05-20 at 12:38 -0400, Dave Reisner wrote:
> On Sun, May 20, 2012 at 06:05:12PM +0200, Davidlohr Bueso wrote:
> > On Sun, 2012-05-20 at 10:51 -0400, Dave Reisner wrote:
> > > On Sun, May 20, 2012 at 04:06:00PM +0200, Davidlohr Bueso wrote:
> > > > On Sat, 2012-05-19 at 20:39 -0400, Dave Reisner wrote:
> > > > > While we're at it, expand this list of known pseudofs types.
> > > > > 
> > > > > Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> > > > > ---
> > > > >  libmount/src/utils.c |   68 +++++++++++++++++++++++++++++++-------------------
> > > > >  1 file changed, 43 insertions(+), 25 deletions(-)
> > > > > 
> > > > > diff --git a/libmount/src/utils.c b/libmount/src/utils.c
> > > > > index b824edc..77557f2 100644
> > > > > --- a/libmount/src/utils.c
> > > > > +++ b/libmount/src/utils.c
> > > > > @@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
> > > > >  	return rc;
> > > > >  }
> > > > >  
> > > > > +/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
> > > > > +static int fstype_cmp(const void *v1, const void *v2)
> > > > > +{
> > > > > +	const char *s1 = *(const char **)v1;
> > > > > +	const char *s2 = *(const char **)v2;
> > > > > +
> > > > > +	return strcmp(s1, s2);
> > > > > +}
> > > > > +
> > > > >  /* returns basename and keeps dirname in the @path, if @path is "/" (root)
> > > > >   * then returns empty string */
> > > > >  static char *stripoff_last_component(char *path)
> > > > > @@ -214,31 +223,40 @@ char *mnt_unmangle(const char *str)
> > > > >   */
> > > > >  int mnt_fstype_is_pseudofs(const char *type)
> > > > >  {
> > > > > -	if (!type)
> > > > > -		return 0;
> > > > > -	if (strcmp(type, "none")  == 0 ||
> > > > > -	    strcmp(type, "proc")  == 0 ||
> > > > > -	    strcmp(type, "tmpfs") == 0 ||
> > > > > -	    strcmp(type, "sysfs") == 0 ||
> > > > > -	    strcmp(type, "autofs") == 0 ||
> > > > > -	    strcmp(type, "devpts") == 0||
> > > > > -	    strcmp(type, "cgroup") == 0 ||
> > > > > -	    strcmp(type, "devtmpfs") == 0 ||
> > > > > -	    strcmp(type, "devfs") == 0 ||
> > > > > -	    strcmp(type, "dlmfs") == 0 ||
> > > > > -	    strcmp(type, "cpuset") == 0 ||
> > > > > -	    strcmp(type, "configfs") == 0 ||
> > > > > -	    strcmp(type, "securityfs") == 0 ||
> > > > > -	    strcmp(type, "hugetlbfs") == 0 ||
> > > > > -	    strcmp(type, "rpc_pipefs") == 0 ||
> > > > > -	    strcmp(type, "fusectl") == 0 ||
> > > > > -	    strcmp(type, "mqueue") == 0 ||
> > > > > -	    strcmp(type, "binfmt_misc") == 0 ||
> > > > > -	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
> > > > > -	    strcmp(type, "debugfs") == 0 ||
> > > > > -	    strcmp(type, "spufs") == 0)
> > > > > -		return 1;
> > > > > -	return 0;
> > > > > +	static const char *pseudofs[] = {
> > > > > +		"anon_inodefs",
> > > > > +		"autofs",
> > > > > +		"bdev",
> > > > > +		"binfmt_misc",
> > > > > +		"cgroup",
> > > > > +		"configfs",
> > > > > +		"cpuset",
> > > > > +		"debugfs",
> > > > > +		"devfs",
> > > > > +		"devpts",
> > > > > +		"devtmpfs",
> > > > > +		"dlmfs",
> > > > > +		"fuse.gvfs-fuse-daemon",
> > > > > +		"fusectl",
> > > > > +		"hugetlbfs",
> > > > > +		"mqueue",
> > > > > +		"nfsd",
> > > > > +		"none",
> > > > > +		"pipefs",
> > > > > +		"proc",
> > > > > +		"pstore",
> > > > > +		"ramfs",
> > > > > +		"rootfs",
> > > > > +		"rpc_pipefs",
> > > > > +		"securityfs",
> > > > > +		"sockfs",
> > > > > +		"spufs",
> > > > > +		"sysfs",
> > > > > +		"tmpfs"
> > > > > +	};
> > > > > +
> > > > 
> > > > What about applying sort (qsort(3)) before searching here? This would
> > > > correct any manual string sorting that pseudofs[] imposes to the
> > > > developer.
> > > > 
> > > 
> > > Hmm, so the overhead from this seems to be negligible, but given that
> > > this is running as part of a library, wouldn't we need a reentrant
> > > version of qsort? qsort_r was added to glibc in 2.8, but I'm not
> > > familiar with how portable this would be to other libc's.
> > 
> > I normally worry about reentrancy when dealing with interrupt handling,
> > like signals. Also consider that pseudofs[] isn't global.
> > 
> 
> qsort_r takes an extra param to the callback to hold an element while
> its being swapped. The reentrancy of the function isn't guaranteed just
> because it operates on a non-global var.
> 
> Regardless, I'm not really in favor of adding this. I think it's reasonable
> to push a little responsibility on the developer and preemptively ensure
> that this is sorted. Perhaps add a comment in the function would be
> sufficient: "make sure this is sorted or Karel will come after you".

Yep, adding a comment to keep it sorted would be enough.

> 
> d
> 
> > > 
> > > > > +	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
> > > > > +				sizeof(char*), fstype_cmp) == NULL);
> > > > >  }
> > > > >  
> > > > >  /**
> > > > 
> > > > 
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe util-linux" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > 
> > 
> 

[PATCH] libmount/utils: Use binary search to compare pseudofs

[Dave Reisner]

It's the responsibility of anyone adding to this list in the future to
ensure that the list remains sorted.

While we're at it, expand this list of known pseudofs types.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
 libmount/src/utils.c |   69 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/libmount/src/utils.c b/libmount/src/utils.c
index b824edc..336fe8d 100644
--- a/libmount/src/utils.c
+++ b/libmount/src/utils.c
@@ -72,6 +72,15 @@ int mnt_parse_offset(const char *str, size_t len, uintmax_t *res)
 	return rc;
 }
 
+/* used as a callback by bsearch in mnt_fstype_is_pseudofs() */
+static int fstype_cmp(const void *v1, const void *v2)
+{
+	const char *s1 = *(const char **)v1;
+	const char *s2 = *(const char **)v2;
+
+	return strcmp(s1, s2);
+}
+
 /* returns basename and keeps dirname in the @path, if @path is "/" (root)
  * then returns empty string */
 static char *stripoff_last_component(char *path)
@@ -214,31 +223,41 @@ char *mnt_unmangle(const char *str)
  */
 int mnt_fstype_is_pseudofs(const char *type)
 {
-	if (!type)
-		return 0;
-	if (strcmp(type, "none")  == 0 ||
-	    strcmp(type, "proc")  == 0 ||
-	    strcmp(type, "tmpfs") == 0 ||
-	    strcmp(type, "sysfs") == 0 ||
-	    strcmp(type, "autofs") == 0 ||
-	    strcmp(type, "devpts") == 0||
-	    strcmp(type, "cgroup") == 0 ||
-	    strcmp(type, "devtmpfs") == 0 ||
-	    strcmp(type, "devfs") == 0 ||
-	    strcmp(type, "dlmfs") == 0 ||
-	    strcmp(type, "cpuset") == 0 ||
-	    strcmp(type, "configfs") == 0 ||
-	    strcmp(type, "securityfs") == 0 ||
-	    strcmp(type, "hugetlbfs") == 0 ||
-	    strcmp(type, "rpc_pipefs") == 0 ||
-	    strcmp(type, "fusectl") == 0 ||
-	    strcmp(type, "mqueue") == 0 ||
-	    strcmp(type, "binfmt_misc") == 0 ||
-	    strcmp(type, "fuse.gvfs-fuse-daemon") == 0 ||
-	    strcmp(type, "debugfs") == 0 ||
-	    strcmp(type, "spufs") == 0)
-		return 1;
-	return 0;
+	/* This array must remain sorted when adding new fstypes */
+	static const char *pseudofs[] = {
+		"anon_inodefs",
+		"autofs",
+		"bdev",
+		"binfmt_misc",
+		"cgroup",
+		"configfs",
+		"cpuset",
+		"debugfs",
+		"devfs",
+		"devpts",
+		"devtmpfs",
+		"dlmfs",
+		"fuse.gvfs-fuse-daemon",
+		"fusectl",
+		"hugetlbfs",
+		"mqueue",
+		"nfsd",
+		"none",
+		"pipefs",
+		"proc",
+		"pstore",
+		"ramfs",
+		"rootfs",
+		"rpc_pipefs",
+		"securityfs",
+		"sockfs",
+		"spufs",
+		"sysfs",
+		"tmpfs"
+	};
+
+	return !(bsearch(&type, pseudofs, ARRAY_SIZE(pseudofs),
+				sizeof(char*), fstype_cmp) == NULL);
 }
 
 /**
-- 
1.7.10.2

Re: [PATCH 1/3] findmnt: reference correct manual section

[Karel Zak]

On Sat, May 19, 2012 at 08:39:19PM -0400, Dave Reisner wrote:
>  misc-utils/findmnt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

 Applied, thanks.

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

Re: [PATCH 2/3] findmnt: fallback to mountinfo for polling

[Karel Zak]

On Sat, May 19, 2012 at 08:39:20PM -0400, Dave Reisner wrote:
>  misc-utils/findmnt.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

 Applied, thanks.

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

Re: [PATCH] libmount/utils: Use binary search to compare pseudofs

[Karel Zak]

On Sun, May 20, 2012 at 12:57:27PM -0400, Dave Reisner wrote:
>  libmount/src/utils.c |   69 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 44 insertions(+), 25 deletions(-)

 Good idea. Applied, thanks.

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