* [PATCH] mountpoint: symlinks are not mount points @ 2014-08-21 18:03 Sami Kerola 2014-08-21 19:28 ` Bernhard Voelker 0 siblings, 1 reply; 4+ messages in thread From: Sami Kerola @ 2014-08-21 18:03 UTC (permalink / raw) To: util-linux; +Cc: kerolasa Hello, Another small change on top of Daves earlier mountpoint(1) correction. Now when mount points are not restricted to directories it is reasonable not to follow symlinks. Notice that this is not a regression from Daves change but something the command has always done. --->8---- From: Sami Kerola <kerolasa@iki.fi> Date: Thu, 21 Aug 2014 20:16:30 +0300 Subject: [PATCH] mountpoint: symlinks are not mount points Earlier the mountpoint(1) followed symlinks, claiming they were mount points when they only referred one. $ ls -l /tmp/this-is-symlink lrwxrwxrwx 1 kerolasa kerolasa 1 Aug 21 19:54 /tmp/this-is-symlink -> / $ mountpoint /tmp/this-is-symlink /tmp/this-is-symlink is a mountpoint Signed-off-by: Sami Kerola <kerolasa@iki.fi> --- sys-utils/mountpoint.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c index 3919ab7..11a611f 100644 --- a/sys-utils/mountpoint.c +++ b/sys-utils/mountpoint.c @@ -182,7 +182,7 @@ int main(int argc, char **argv) spec = argv[optind++]; - if (stat(spec, &st)) { + if (lstat(spec, &st)) { if (!quiet) err(EXIT_FAILURE, "%s", spec); return EXIT_FAILURE; @@ -192,7 +192,7 @@ int main(int argc, char **argv) else { dev_t src; - if ( dir_to_device(spec, &src)) { + if (dir_to_device(spec, &src) || S_ISLNK(st.st_mode)) { if (!quiet) printf(_("%s is not a mountpoint\n"), spec); return EXIT_FAILURE; -- 2.1.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mountpoint: symlinks are not mount points 2014-08-21 18:03 [PATCH] mountpoint: symlinks are not mount points Sami Kerola @ 2014-08-21 19:28 ` Bernhard Voelker 2014-08-21 21:34 ` Sami Kerola 0 siblings, 1 reply; 4+ messages in thread From: Bernhard Voelker @ 2014-08-21 19:28 UTC (permalink / raw) To: Sami Kerola, util-linux On 08/21/2014 08:03 PM, Sami Kerola wrote: > Earlier the mountpoint(1) followed symlinks IMO this is correct as symlinks only have to be treated as such when explicitly not de-referencing. As mountpoint(1) does not have a --no-dereference option, symlinks should always be transparent for this tool. Furthermore, the patch would break scripts relying on existing behavior: $ ln -s / slink $ /usr/bin/mountpoint slink slink is a mountpoint $ # ~berny/util-linux/mountpoint slink slink is not a mountpoint Have a nice day, Berny ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mountpoint: symlinks are not mount points 2014-08-21 19:28 ` Bernhard Voelker @ 2014-08-21 21:34 ` Sami Kerola 2014-08-22 6:34 ` Karel Zak 0 siblings, 1 reply; 4+ messages in thread From: Sami Kerola @ 2014-08-21 21:34 UTC (permalink / raw) To: util-linux; +Cc: kerolasa, Bernhard Voelker On 21 August 2014 22:28, Bernhard Voelker <mail@bernhard-voelker.de> wrote: > On 08/21/2014 08:03 PM, Sami Kerola wrote: >> Earlier the mountpoint(1) followed symlinks > > IMO this is correct as symlinks only have to be treated > as such when explicitly not de-referencing. > As mountpoint(1) does not have a --no-dereference option, > symlinks should always be transparent for this tool. > > Furthermore, the patch would break scripts relying on existing > behavior: > > $ ln -s / slink > > $ /usr/bin/mountpoint slink > slink is a mountpoint > > $ # ~berny/util-linux/mountpoint slink > slink is not a mountpoint Hi Berny, I don't share view symlinks are mount points. In my mind they are pointers, much like street signs, giving a direction to somewhere without understanding what is there if anything at all. But not breaking existing behavior is good point, so I changed the patch and added --no-dereference. --->8---- From: Sami Kerola <kerolasa@iki.fi> Date: Fri, 22 Aug 2014 00:16:57 +0300 Subject: [PATCH] mountpoint: add --no-dereference option By default the mountpoint follows symlinks. $ ls -l /tmp/this-is-symlink lrwxrwxrwx 1 kerolasa kerolasa 1 Aug 21 19:54 /tmp/this-is-symlink -> / $ mountpoint /tmp/this-is-symlink /tmp/this-is-symlink is a mountpoint When use --no-dereference is used symlinks will not be reported as mount points, even when they are pointing to such. CC: Bernhard Voelker <mail@bernhard-voelker.de> Signed-off-by: Sami Kerola <kerolasa@iki.fi> --- sys-utils/mountpoint.1 | 3 +++ sys-utils/mountpoint.c | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/sys-utils/mountpoint.1 b/sys-utils/mountpoint.1 index 29c153c..c99ba41 100644 --- a/sys-utils/mountpoint.1 +++ b/sys-utils/mountpoint.1 @@ -28,6 +28,9 @@ Be quiet - don't print anything. .BR \-x , " \-\-devno" Show the major/minor numbers of the given blockdevice on standard output. .TP +.BR \-\-no\-dereference +Do not follow symbolic links. +.TP .BR \-V , " \-\-version" Display version information and exit. .TP diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c index 3919ab7..c34fbee 100644 --- a/sys-utils/mountpoint.c +++ b/sys-utils/mountpoint.c @@ -123,6 +123,7 @@ static void __attribute__((__noreturn__)) usage(FILE *out) fputs(_(" -q, --quiet quiet mode - don't print anything\n" " -d, --fs-devno print maj:min device number of the filesystem\n" " -x, --devno print maj:min device number of the block device\n"), out); + fputs(_(" --no-dereference never follow symbolic links\n"), out); fputs(USAGE_SEPARATOR, out); fputs(USAGE_HELP, out); fputs(USAGE_VERSION, out); @@ -133,14 +134,18 @@ static void __attribute__((__noreturn__)) usage(FILE *out) int main(int argc, char **argv) { - int c, fs_devno = 0, dev_devno = 0, rc = 0; + int c, fs_devno = 0, dev_devno = 0, no_dereference = 0, rc = 0; char *spec; struct stat st; + enum { + OPT_NO_DEREFERENCE = CHAR_MAX + 1 + }; static const struct option longopts[] = { { "quiet", 0, 0, 'q' }, { "fs-devno", 0, 0, 'd' }, { "devno", 0, 0, 'x' }, + { "no-dereference", 0, 0, OPT_NO_DEREFERENCE}, { "help", 0, 0, 'h' }, { "version", 0, 0, 'V' }, { NULL, 0, 0, 0 } @@ -165,6 +170,9 @@ int main(int argc, char **argv) case 'x': dev_devno = 1; break; + case OPT_NO_DEREFERENCE: + no_dereference = 1; + break; case 'h': usage(stdout); break; @@ -182,7 +190,7 @@ int main(int argc, char **argv) spec = argv[optind++]; - if (stat(spec, &st)) { + if (lstat(spec, &st)) { if (!quiet) err(EXIT_FAILURE, "%s", spec); return EXIT_FAILURE; @@ -192,7 +200,7 @@ int main(int argc, char **argv) else { dev_t src; - if ( dir_to_device(spec, &src)) { + if (dir_to_device(spec, &src) || (no_dereference && S_ISLNK(st.st_mode))) { if (!quiet) printf(_("%s is not a mountpoint\n"), spec); return EXIT_FAILURE; -- 2.1.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mountpoint: symlinks are not mount points 2014-08-21 21:34 ` Sami Kerola @ 2014-08-22 6:34 ` Karel Zak 0 siblings, 0 replies; 4+ messages in thread From: Karel Zak @ 2014-08-22 6:34 UTC (permalink / raw) To: Sami Kerola; +Cc: util-linux, Bernhard Voelker On Fri, Aug 22, 2014 at 12:34:27AM +0300, Sami Kerola wrote: > On 21 August 2014 22:28, Bernhard Voelker <mail@bernhard-voelker.de> wrote: > > On 08/21/2014 08:03 PM, Sami Kerola wrote: > >> Earlier the mountpoint(1) followed symlinks > > > > IMO this is correct as symlinks only have to be treated > > as such when explicitly not de-referencing. > > As mountpoint(1) does not have a --no-dereference option, > > symlinks should always be transparent for this tool. > > > > Furthermore, the patch would break scripts relying on existing > > behavior: > > > > $ ln -s / slink > > > > $ /usr/bin/mountpoint slink > > slink is a mountpoint > > > > $ # ~berny/util-linux/mountpoint slink > > slink is not a mountpoint > > I don't share view symlinks are mount points. In my mind they are > pointers, much like street signs, giving a direction to somewhere without > understanding what is there if anything at all. But not breaking > existing behavior is good point, so I changed the patch and added > --no-dereference. I don't think we need the patch, it does not resolve or improve anything. It's over-engineering. If you don't like symlinks then you can use if [ ! -L $dir ]; then mountpoint $dir fi > - if (stat(spec, &st)) { > + if (lstat(spec, &st)) { btw, you want to use lstat() only when no_dereference is specified. Anyway, NACK. Sorry. Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-08-22 6:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-21 18:03 [PATCH] mountpoint: symlinks are not mount points Sami Kerola 2014-08-21 19:28 ` Bernhard Voelker 2014-08-21 21:34 ` Sami Kerola 2014-08-22 6:34 ` Karel Zak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox