* [PATCH 1/2] mountpoint: refactor exit path
@ 2011-10-09 15:12 Dave Reisner
2011-10-09 15:12 ` [PATCH 2/2] mountpoint: support symbolic and relative paths Dave Reisner
2011-10-10 19:45 ` [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
0 siblings, 2 replies; 6+ messages in thread
From: Dave Reisner @ 2011-10-09 15:12 UTC (permalink / raw)
To: util-linux; +Cc: Dave Reisner
There's only one condition for which we declare success, but many for
failure. Initialize rc as failure and set to success on this single
condition. In all cases, jump to a label to exit instead of exiting
immediately. This will be used later on to ease cleanup of any heap
allocations.
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
sys-utils/mountpoint.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index 1297d82..e021c70 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -105,7 +105,7 @@ 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, rc = EXIT_FAILURE;
char *spec;
struct stat st;
@@ -152,7 +152,7 @@ int main(int argc, char **argv)
if (stat(spec, &st)) {
if (!quiet)
err(EXIT_FAILURE, "%s", spec);
- return EXIT_FAILURE;
+ goto finish;
}
if (dev_devno)
rc = print_devno(spec, &st);
@@ -162,19 +162,22 @@ int main(int argc, char **argv)
if (!S_ISDIR(st.st_mode)) {
if (!quiet)
errx(EXIT_FAILURE, _("%s: not a directory"), spec);
- return EXIT_FAILURE;
+ goto finish;
}
src = dir_to_device(spec);
if (src == (dev_t)-1) {
if (!quiet)
printf(_("%s is not a mountpoint\n"), spec);
- return EXIT_FAILURE;
+ goto finish;
}
if (fs_devno)
printf("%u:%u\n", major(src), minor(src));
- else if (!quiet)
+ else if (!quiet) {
printf(_("%s is a mountpoint\n"), spec);
+ rc = EXIT_SUCCESS;
+ }
}
- return rc ? EXIT_FAILURE : EXIT_SUCCESS;
+finish:
+ return rc;
}
--
1.7.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] mountpoint: support symbolic and relative paths
2011-10-09 15:12 [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
@ 2011-10-09 15:12 ` Dave Reisner
2011-10-10 19:45 ` [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
1 sibling, 0 replies; 6+ messages in thread
From: Dave Reisner @ 2011-10-09 15:12 UTC (permalink / raw)
To: util-linux; +Cc: Dave Reisner
Use our own canonicalize library to resolve a provided path before
checking if its a mountpoint.
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
Unless I've missed something, this should bring u-l's mountpoint to feature
parity with sysvinit's.
sys-utils/Makefile.am | 1 +
sys-utils/mountpoint.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/sys-utils/Makefile.am b/sys-utils/Makefile.am
index 8059ee6..34108da 100644
--- a/sys-utils/Makefile.am
+++ b/sys-utils/Makefile.am
@@ -44,6 +44,7 @@ flock_SOURCES = flock.c $(top_srcdir)/lib/strutils.c
if BUILD_MOUNTPOINT
bin_PROGRAMS += mountpoint
+mountpoint_SOURCES = mountpoint.c $(top_srcdir)/lib/canonicalize.c
mountpoint_LDADD = $(ul_libmount_la)
mountpoint_CFLAGS = $(AM_CFLAGS) -I$(ul_libmount_incdir)
dist_man_MANS += mountpoint.1
diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index e021c70..204f061 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -37,6 +37,7 @@
#include "nls.h"
#include "xalloc.h"
#include "c.h"
+#include "canonicalize.h"
static int quiet;
@@ -147,7 +148,8 @@ int main(int argc, char **argv)
if (optind + 1 != argc)
usage(stderr);
- spec = argv[optind++];
+ /* resolve relative paths to absolute */
+ spec = canonicalize_path(argv[optind++]);
if (stat(spec, &st)) {
if (!quiet)
@@ -179,5 +181,6 @@ int main(int argc, char **argv)
}
finish:
+ free(spec);
return rc;
}
--
1.7.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mountpoint: refactor exit path
2011-10-09 15:12 [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
2011-10-09 15:12 ` [PATCH 2/2] mountpoint: support symbolic and relative paths Dave Reisner
@ 2011-10-10 19:45 ` Dave Reisner
2011-10-10 23:05 ` Karel Zak
1 sibling, 1 reply; 6+ messages in thread
From: Dave Reisner @ 2011-10-10 19:45 UTC (permalink / raw)
To: util-linux
On Sun, Oct 09, 2011 at 11:12:04AM -0400, Dave Reisner wrote:
> There's only one condition for which we declare success, but many for
> failure. Initialize rc as failure and set to success on this single
> condition. In all cases, jump to a label to exit instead of exiting
> immediately. This will be used later on to ease cleanup of any heap
> allocations.
>
> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> ---
Noticing that this could be done better -- the return/goto calls here
don't do anything because err/errx are exiting after throwing the error
message. They should be replaced with equivalent warn/warnx calls so we
hit the goto on error and cleanup before exiting.
@Karel: I can resend this if you like.
Regards,
dave
> sys-utils/mountpoint.c | 15 +++++++++------
> 1 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
> index 1297d82..e021c70 100644
> --- a/sys-utils/mountpoint.c
> +++ b/sys-utils/mountpoint.c
> @@ -105,7 +105,7 @@ 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, rc = EXIT_FAILURE;
> char *spec;
> struct stat st;
>
> @@ -152,7 +152,7 @@ int main(int argc, char **argv)
> if (stat(spec, &st)) {
> if (!quiet)
> err(EXIT_FAILURE, "%s", spec);
> - return EXIT_FAILURE;
> + goto finish;
> }
> if (dev_devno)
> rc = print_devno(spec, &st);
> @@ -162,19 +162,22 @@ int main(int argc, char **argv)
> if (!S_ISDIR(st.st_mode)) {
> if (!quiet)
> errx(EXIT_FAILURE, _("%s: not a directory"), spec);
> - return EXIT_FAILURE;
> + goto finish;
> }
> src = dir_to_device(spec);
> if (src == (dev_t)-1) {
> if (!quiet)
> printf(_("%s is not a mountpoint\n"), spec);
> - return EXIT_FAILURE;
> + goto finish;
> }
> if (fs_devno)
> printf("%u:%u\n", major(src), minor(src));
> - else if (!quiet)
> + else if (!quiet) {
> printf(_("%s is a mountpoint\n"), spec);
> + rc = EXIT_SUCCESS;
> + }
> }
>
> - return rc ? EXIT_FAILURE : EXIT_SUCCESS;
> +finish:
> + return rc;
> }
> --
> 1.7.7
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mountpoint: refactor exit path
2011-10-10 19:45 ` [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
@ 2011-10-10 23:05 ` Karel Zak
0 siblings, 0 replies; 6+ messages in thread
From: Karel Zak @ 2011-10-10 23:05 UTC (permalink / raw)
To: Dave Reisner; +Cc: util-linux
On Mon, Oct 10, 2011 at 03:45:45PM -0400, Dave Reisner wrote:
> On Sun, Oct 09, 2011 at 11:12:04AM -0400, Dave Reisner wrote:
> > There's only one condition for which we declare success, but many for
> > failure. Initialize rc as failure and set to success on this single
> > condition. In all cases, jump to a label to exit instead of exiting
> > immediately. This will be used later on to ease cleanup of any heap
> > allocations.
Well, free() before exit() is usually waste of time... but maybe we
can think about mountpoint(1) as about libmount test application,
then it would be nice to keep it without leaks (for example for
valgrind).
> @Karel: I can resend this if you like.
go ahead, thanks
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] mountpoint: support symbolic and relative paths
2011-10-10 23:14 Dave Reisner
@ 2011-10-10 23:14 ` Dave Reisner
2011-10-11 9:34 ` Karel Zak
0 siblings, 1 reply; 6+ messages in thread
From: Dave Reisner @ 2011-10-10 23:14 UTC (permalink / raw)
To: util-linux; +Cc: Dave Reisner
Use our own canonicalize library to resolve a provided path before
checking if its a mountpoint.
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
sys-utils/Makefile.am | 1 +
sys-utils/mountpoint.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/sys-utils/Makefile.am b/sys-utils/Makefile.am
index cfaf053..49141fb 100644
--- a/sys-utils/Makefile.am
+++ b/sys-utils/Makefile.am
@@ -44,6 +44,7 @@ flock_SOURCES = flock.c $(top_srcdir)/lib/strutils.c
if BUILD_MOUNTPOINT
bin_PROGRAMS += mountpoint
+mountpoint_SOURCES = mountpoint.c $(top_srcdir)/lib/canonicalize.c
mountpoint_LDADD = $(ul_libmount_la)
mountpoint_CFLAGS = $(AM_CFLAGS) -I$(ul_libmount_incdir)
dist_man_MANS += mountpoint.1
diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index e075b83..2565107 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -37,6 +37,7 @@
#include "nls.h"
#include "xalloc.h"
#include "c.h"
+#include "canonicalize.h"
static int quiet;
@@ -147,7 +148,8 @@ int main(int argc, char **argv)
if (optind + 1 != argc)
usage(stderr);
- spec = argv[optind++];
+ /* resolve relative paths to absolute */
+ spec = canonicalize_path(argv[optind++]);
if (stat(spec, &st)) {
if (!quiet)
@@ -179,5 +181,6 @@ int main(int argc, char **argv)
}
finish:
+ free(spec);
return rc;
}
--
1.7.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mountpoint: support symbolic and relative paths
2011-10-10 23:14 ` [PATCH 2/2] mountpoint: support symbolic and relative paths Dave Reisner
@ 2011-10-11 9:34 ` Karel Zak
0 siblings, 0 replies; 6+ messages in thread
From: Karel Zak @ 2011-10-11 9:34 UTC (permalink / raw)
To: Dave Reisner; +Cc: util-linux, Dave Reisner
On Mon, Oct 10, 2011 at 07:14:16PM -0400, Dave Reisner wrote:
> Use our own canonicalize library to resolve a provided path before
> checking if its a mountpoint.
Good idea. Thanks.
I have applied a more robust solution based on libmount.
Karel
==27191== Command:
/home/projects/util-linux/util-linux/sys-utils/.libs/mountpoint home
==27191==
home is a mountpoint
==27191==
==27191== HEAP SUMMARY:
==27191== in use at exit: 0 bytes in 0 blocks
==27191== total heap usage: 535 allocs, 535 frees, 44,094 bytes
allocated
==27191==
==27191== All heap blocks were freed -- no leaks are possible
==27191==
==27191== For counts of detected and suppressed errors, rerun with: -v
==27191== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-11 9:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-09 15:12 [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
2011-10-09 15:12 ` [PATCH 2/2] mountpoint: support symbolic and relative paths Dave Reisner
2011-10-10 19:45 ` [PATCH 1/2] mountpoint: refactor exit path Dave Reisner
2011-10-10 23:05 ` Karel Zak
-- strict thread matches above, loose matches on Subject: below --
2011-10-10 23:14 Dave Reisner
2011-10-10 23:14 ` [PATCH 2/2] mountpoint: support symbolic and relative paths Dave Reisner
2011-10-11 9: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;
as well as URLs for NNTP newsgroup(s).