[PATCH 1/3] mkfs: drop hardcoded search path

[PATCH 1/3] mkfs: drop hardcoded search path

[Mike Frysinger]

Rather than maintain a hardcoded search path for looking up tools and
override the user's PATH env setting, respect whatever the user has.
This matches the convention of just about every other tool out there.

It might break on systems that don't have /sbin in their PATH and they
try to run /sbin/mkfs directly, but so be it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 disk-utils/mkfs.8 | 16 ++--------------
 disk-utils/mkfs.c | 17 +----------------
 2 files changed, 3 insertions(+), 30 deletions(-)

diff --git a/disk-utils/mkfs.8 b/disk-utils/mkfs.8
index 9a5cdc0..0279260 100644
--- a/disk-utils/mkfs.8
+++ b/disk-utils/mkfs.8
@@ -29,20 +29,8 @@ In actuality,
 is simply a front-end for the various filesystem builders
 (\fBmkfs.\fIfstype\fR)
 available under Linux.
-The filesystem-specific builder is searched for in a number
-of directories, like perhaps
-.IR /sbin ,
-.IR /sbin/fs ,
-.IR /sbin/fs.d ,
-.IR /etc/fs ,
-.I /etc
-(the precise list is defined at compile time but at least
-contains
-.I /sbin
-and
-.IR /sbin/fs ),
-and finally in the directories
-listed in the PATH environment variable.
+The filesystem-specific builder is searched for via your PATH
+environment setting only.
 Please see the filesystem-specific builder manual pages for
 further details.
 .SH OPTIONS
diff --git a/disk-utils/mkfs.c b/disk-utils/mkfs.c
index 08c4ef0..cf1a312 100644
--- a/disk-utils/mkfs.c
+++ b/disk-utils/mkfs.c
@@ -38,10 +38,6 @@
 #define DEFAULT_FSTYPE	"ext2"
 #endif
 
-#define SEARCH_PATH	"PATH=" FS_SEARCH_PATH
-#define PROGNAME	"mkfs.%s"
-
-
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
 	fputs(USAGE_HEADER, out);
@@ -78,7 +74,6 @@ int main(int argc, char **argv)
 	char *progname;		/* name of executable to be called */
 	char *fstype = NULL;
 	int i, more = 0, verbose = 0;
-	char *oldpath, *newpath;
 
 	enum { VERSION_OPTION = CHAR_MAX + 1 };
 
@@ -126,17 +121,7 @@ int main(int argc, char **argv)
 	if (fstype == NULL)
 		fstype = DEFAULT_FSTYPE;
 
-	/* Set PATH and program name */
-	oldpath = getenv("PATH");
-	if (!oldpath)
-		oldpath = "/bin";
-
-	newpath = xmalloc(strlen(oldpath) + sizeof(SEARCH_PATH) + 3);
-	sprintf(newpath, "%s:%s\n", SEARCH_PATH, oldpath);
-	putenv(newpath);
-
-	progname = xmalloc(sizeof(PROGNAME) + strlen(fstype) + 1);
-	sprintf(progname, PROGNAME, fstype);
+	xasprintf(&progname, "mkfs.%s", fstype);
 	argv[--optind] = progname;
 
 	if (verbose) {
-- 
2.3.5

[PATCH 2/3] swapon: search for mkswap via PATH

[Mike Frysinger]

Rather than hardcode /sbin/mkswap all the time, use a normal PATH search.
This matches the normal behavior of other tools, and makes local testing
easier.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 sys-utils/swapon.8 | 2 +-
 sys-utils/swapon.c | 6 ++----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/sys-utils/swapon.8 b/sys-utils/swapon.8
index e571a5f..299da22 100644
--- a/sys-utils/swapon.8
+++ b/sys-utils/swapon.8
@@ -108,7 +108,7 @@ may also be used to skip non-existing device.
 
 .TP
 .BR \-f , " \-\-fixpgsz"
-Reinitialize (exec /sbin/mkswap) the swap space if its page size does not
+Reinitialize (exec mkswap) the swap space if its page size does not
 match that of the current running kernel.
 .BR mkswap (2)
 initializes the whole device and does not check for bad blocks.
diff --git a/sys-utils/swapon.c b/sys-utils/swapon.c
index 645a01d..561b1a9 100644
--- a/sys-utils/swapon.c
+++ b/sys-utils/swapon.c
@@ -28,8 +28,6 @@
 #include "swapprober.h"
 #include "swapon-common.h"
 
-#define PATH_MKSWAP	"/sbin/mkswap"
-
 #ifdef HAVE_SYS_SWAP_H
 # include <sys/swap.h>
 #endif
@@ -303,7 +301,7 @@ static int swap_reinitialize(const char *device,
 		return -1;
 
 	case 0:	/* child */
-		cmd[idx++] = PATH_MKSWAP;
+		cmd[idx++] = "mkswap";
 		if (label && *label) {
 			cmd[idx++] = "-L";
 			cmd[idx++] = (char *) label;
@@ -314,7 +312,7 @@ static int swap_reinitialize(const char *device,
 		}
 		cmd[idx++] = (char *) device;
 		cmd[idx++] = NULL;
-		execv(cmd[0], cmd);
+		execvp(cmd[0], cmd);
 		err(EXIT_FAILURE, _("failed to execute %s"), cmd[0]);
 
 	default: /* parent */
-- 
2.3.5

Re: [PATCH 2/3] swapon: search for mkswap via PATH

[Karel Zak]

On Sun, Apr 12, 2015 at 05:06:52AM -0400, Mike Frysinger wrote:
>  sys-utils/swapon.8 | 2 +-
>  sys-utils/swapon.c | 6 ++----
>  2 files changed, 3 insertions(+), 5 deletions(-)

 Applied, thanks.

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

[PATCH 3/3] fsck: drop hardcoded search path

[Mike Frysinger]

Rather than maintain a hardcoded search path for looking up tools and
override the user's PATH env setting, respect whatever the user has.
This matches the convention of just about every other tool out there.

It might break on systems that don't have /sbin in their PATH and they
try to run /sbin/fsck directly, or are using the uncommon fs.d subdirs,
but so be it.

We also drop the hardcoded minimal list of filesystem types that we
"really want" to check.  Instead, we blindly execute the fsck.<type>
program whenever it's been requested.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 disk-utils/fsck.8 |  23 +++---------
 disk-utils/fsck.c | 103 ++++++------------------------------------------------
 2 files changed, 14 insertions(+), 112 deletions(-)

diff --git a/disk-utils/fsck.8 b/disk-utils/fsck.8
index c60eef2..4207731 100644
--- a/disk-utils/fsck.8
+++ b/disk-utils/fsck.8
@@ -80,14 +80,9 @@ In actuality,
 .B fsck
 is simply a front-end for the various filesystem checkers
 (\fBfsck\fR.\fIfstype\fR) available under Linux.  The
-filesystem-specific checker is searched for in
-.I /sbin
-first, then in
-.I /etc/fs
-and
-.IR /etc ,
-and finally in the directories listed in the PATH environment
-variable.  Please see the filesystem-specific checker manual pages for
+filesystem-specific checker is searched for via the PATH
+environment variable only.
+Please see the filesystem-specific checker manual pages for
 further details.
 .SH OPTIONS
 .TP
@@ -416,17 +411,7 @@ be run based on gathering accounting data from the operating system.
 .B PATH
 The
 .B PATH
-environment variable is used to find filesystem checkers.  A set of
-system directories are searched first:
-.BR /sbin ,
-.BR /sbin/fs.d ,
-.BR /sbin/fs ,
-.BR /etc/fs ,
-and
-.BR /etc .
-Then the set of directories found in the
-.B PATH
-environment are searched.
+environment variable is used to find filesystem checkers.
 .TP
 .B FSTAB_FILE
 This environment variable allows the system administrator
diff --git a/disk-utils/fsck.c b/disk-utils/fsck.c
index 817be97..b1fc187 100644
--- a/disk-utils/fsck.c
+++ b/disk-utils/fsck.c
@@ -75,16 +75,6 @@ static const char *ignored_types[] = {
 	NULL
 };
 
-static const char *really_wanted[] = {
-	"minix",
-	"ext2",
-	"ext3",
-	"ext4",
-	"ext4dev",
-	"jfs",
-	"reiserfs"
-};
-
 /*
  * Internal structure for mount tabel entries.
  */
@@ -150,9 +140,6 @@ static int kill_sent;
 static char *fstype;
 static struct fsck_instance *instance_list;
 
-static const char fsck_prefix_path[] = FS_SEARCH_PATH;
-static char *fsck_path;
-
 /* parsed fstab and mtab */
 static struct libmnt_table *fstab, *mtab;
 static struct libmnt_cache *mntcache;
@@ -171,19 +158,6 @@ static int string_to_int(const char *s)
 		return (int) l;
 }
 
-/* Do we really really want to check this fs? */
-static int fs_check_required(const char *type)
-{
-	size_t i;
-
-	for(i = 0; i < ARRAY_SIZE(really_wanted); i++) {
-		if (strcmp(type, really_wanted[i]) == 0)
-			return 1;
-	}
-
-	return 0;
-}
-
 static int is_mounted(struct libmnt_fs *fs)
 {
 	int rc;
@@ -535,28 +509,6 @@ static struct libmnt_fs *lookup(char *path)
 	return fs;
 }
 
-/* Find fsck program for a given fs type. */
-static char *find_fsck(const char *type)
-{
-	char *s;
-	const char *tpl;
-	static char prog[256];
-	char *p = xstrdup(fsck_path);
-	struct stat st;
-
-	/* Are we looking for a program or just a type? */
-	tpl = (strncmp(type, "fsck.", 5) ? "%s/fsck.%s" : "%s/%s");
-
-	for(s = strtok(p, ":"); s; s = strtok(NULL, ":")) {
-		sprintf(prog, tpl, s, type);
-		if (stat(prog, &st) == 0)
-			break;
-	}
-	free(p);
-
-	return(s ? prog : NULL);
-}
-
 static int progress_active(void)
 {
 	struct fsck_instance *inst;
@@ -601,8 +553,7 @@ static void print_stats(struct fsck_instance *inst)
  * Execute a particular fsck program, and link it into the list of
  * child processes we are waiting for.
  */
-static int execute(const char *progname, const char *progpath,
-		   const char *type, struct libmnt_fs *fs, int interactive)
+static int execute(const char *type, struct libmnt_fs *fs, int interactive)
 {
 	char *argv[80];
 	int  argc, i;
@@ -611,7 +562,7 @@ static int execute(const char *progname, const char *progpath,
 
 	inst = xcalloc(1, sizeof(*inst));
 
-	argv[0] = xstrdup(progname);
+	xasprintf(&argv[0], "fsck.%s", type);
 	argc = 1;
 
 	for (i=0; i <num_args; i++)
@@ -643,7 +594,7 @@ static int execute(const char *progname, const char *progpath,
 
 		if (!tgt)
 			tgt = fs_get_device(fs);
-		printf("[%s (%d) -- %s] ", progpath, num_running, tgt);
+		printf("[%s (%d) -- %s] ", argv[0], num_running, tgt);
 		for (i=0; i < argc; i++)
 			printf("%s ", argv[i]);
 		printf("\n");
@@ -666,19 +617,19 @@ static int execute(const char *progname, const char *progpath,
 	} else if (pid == 0) {
 		if (!interactive)
 			close(0);
-		execv(progpath, argv);
-		err(FSCK_EX_ERROR, _("%s: execute failed"), progpath);
+		execvp(argv[0], argv);
+		err(FSCK_EX_ERROR, _("%s: execute failed"), argv[0]);
 	}
 
-	for (i=0; i < argc; i++)
-		free(argv[i]);
-
 	inst->pid = pid;
-	inst->prog = xstrdup(progname);
+	inst->prog = xstrdup(argv[0]);
 	inst->type = xstrdup(type);
 	gettime_monotonic(&inst->start_time);
 	inst->next = NULL;
 
+	for (i = 0; i < argc; i++)
+		free(argv[i]);
+
 	/*
 	 * Find the end of the list, so we add the instance on at the end.
 	 */
@@ -871,7 +822,6 @@ static int wait_many(int flags)
  */
 static int fsck_device(struct libmnt_fs *fs, int interactive)
 {
-	char progname[80], *progpath;
 	const char *type;
 	int retval;
 
@@ -888,18 +838,8 @@ static int fsck_device(struct libmnt_fs *fs, int interactive)
 	else
 		type = DEFAULT_FSTYPE;
 
-	sprintf(progname, "fsck.%s", type);
-	progpath = find_fsck(progname);
-	if (progpath == NULL) {
-		if (fs_check_required(type)) {
-			retval = ENOENT;
-			goto err;
-		}
-		return 0;
-	}
-
 	num_running++;
-	retval = execute(progname, progpath, type, fs, interactive);
+	retval = execute(type, fs, interactive);
 	if (retval) {
 		num_running--;
 		goto err;
@@ -1131,16 +1071,6 @@ static int ignore(struct libmnt_fs *fs)
 	if (fs_ignored_type(fs))
 		return 1;
 
-
-
-	/* See if the <fsck.fs> program is available. */
-	if (find_fsck(type) == NULL) {
-		if (fs_check_required(type))
-			warnx(_("cannot check %s: fsck.%s not found"),
-				fs_get_device(fs), type);
-		return 1;
-	}
-
 	/* We can and want to check this file system type. */
 	return 0;
 }
@@ -1551,7 +1481,6 @@ int main(int argc, char *argv[])
 {
 	int i, status = 0;
 	int interactive = 0;
-	char *oldpath = getenv("PATH");
 	struct libmnt_fs *fs;
 
 	setvbuf(stdout, NULL, _IONBF, BUFSIZ);
@@ -1573,17 +1502,6 @@ int main(int argc, char *argv[])
 
 	load_fs_info();
 
-	/* Update our search path to include uncommon directories. */
-	if (oldpath) {
-		fsck_path = xmalloc (strlen (fsck_prefix_path) + 1 +
-				    strlen (oldpath) + 1);
-		strcpy (fsck_path, fsck_prefix_path);
-		strcat (fsck_path, ":");
-		strcat (fsck_path, oldpath);
-	} else {
-		fsck_path = xstrdup(fsck_prefix_path);
-	}
-
 	if ((num_devices == 1) || (serialize))
 		interactive = 1;
 
@@ -1632,7 +1550,6 @@ int main(int argc, char *argv[])
 		}
 	}
 	status |= wait_many(FLAG_WAIT_ALL);
-	free(fsck_path);
 	mnt_unref_cache(mntcache);
 	mnt_unref_table(fstab);
 	mnt_unref_table(mtab);
-- 
2.3.5

Re: [PATCH 3/3] fsck: drop hardcoded search path

[Karel Zak]

On Sun, Apr 12, 2015 at 05:06:53AM -0400, Mike Frysinger wrote:
> We also drop the hardcoded minimal list of filesystem types that we
> "really want" to check.  Instead, we blindly execute the fsck.<type>
> program whenever it's been requested.

 This change makes fsck.<type> required for all filesystem, so it
 breaks boot on many systems... Now fsck.<type> is optional for many
 fs types (e.g. btrfs, xfs, ...).

> @@ -888,18 +838,8 @@ static int fsck_device(struct libmnt_fs *fs, int interactive)
>  	else
>  		type = DEFAULT_FSTYPE;
>  
> -	sprintf(progname, "fsck.%s", type);
> -	progpath = find_fsck(progname);
> -	if (progpath == NULL) {
> -		if (fs_check_required(type)) {
> -			retval = ENOENT;
> -			goto err;
> -		}
> -		return 0;
> -	}

 This is important detail.

    Karel

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

Re: [PATCH 3/3] fsck: drop hardcoded search path

[Karel Zak]

On Sun, Apr 12, 2015 at 05:06:53AM -0400, Mike Frysinger wrote:
>  disk-utils/fsck.8 |  23 +++---------
>  disk-utils/fsck.c | 103 ++++++------------------------------------------------
>  2 files changed, 14 insertions(+), 112 deletions(-)

 I have applied a little bit less invasive patch to keep the current
 semantic with required/optional checkers.

    Karel

>From a03bdbcd2029ed1b002d45a028f6e706845fa566 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@redhat.com>
Date: Mon, 27 Apr 2015 10:51:33 +0200
Subject: [PATCH] fsck: use PATH or fallback to /sbin

It's overkill to support all the obscure paths like /sbin/fs.d. We
have PATH for customization, that's enough.

It still seems like a good idea to keep fsck robust, because it's used
by boot scripts/systemd. For this reason fsck fallbacks to "/sbin" if
PATH is undefined or empty.

Signed-off-by: Karel Zak <kzak@redhat.com>
---
 disk-utils/fsck.8 | 25 ++++++-------------------
 disk-utils/fsck.c | 16 ++++------------
 2 files changed, 10 insertions(+), 31 deletions(-)

diff --git a/disk-utils/fsck.8 b/disk-utils/fsck.8
index c60eef2..c2bb7bf 100644
--- a/disk-utils/fsck.8
+++ b/disk-utils/fsck.8
@@ -80,14 +80,11 @@ In actuality,
 .B fsck
 is simply a front-end for the various filesystem checkers
 (\fBfsck\fR.\fIfstype\fR) available under Linux.  The
-filesystem-specific checker is searched for in
-.I /sbin
-first, then in
-.I /etc/fs
-and
-.IR /etc ,
-and finally in the directories listed in the PATH environment
-variable.  Please see the filesystem-specific checker manual pages for
+filesystem-specific checker is searched for in the
+PATH environment variable. If the PATH is undefined then
+fallback to "/sbin".
+.PP
+Please see the filesystem-specific checker manual pages for
 further details.
 .SH OPTIONS
 .TP
@@ -416,17 +413,7 @@ be run based on gathering accounting data from the operating system.
 .B PATH
 The
 .B PATH
-environment variable is used to find filesystem checkers.  A set of
-system directories are searched first:
-.BR /sbin ,
-.BR /sbin/fs.d ,
-.BR /sbin/fs ,
-.BR /etc/fs ,
-and
-.BR /etc .
-Then the set of directories found in the
-.B PATH
-environment are searched.
+environment variable is used to find filesystem checkers.
 .TP
 .B FSTAB_FILE
 This environment variable allows the system administrator
diff --git a/disk-utils/fsck.c b/disk-utils/fsck.c
index 817be97..299a775 100644
--- a/disk-utils/fsck.c
+++ b/disk-utils/fsck.c
@@ -150,9 +150,10 @@ static int kill_sent;
 static char *fstype;
 static struct fsck_instance *instance_list;
 
-static const char fsck_prefix_path[] = FS_SEARCH_PATH;
+#define FSCK_DEFAULT_PATH "/sbin"
 static char *fsck_path;
 
+
 /* parsed fstab and mtab */
 static struct libmnt_table *fstab, *mtab;
 static struct libmnt_cache *mntcache;
@@ -1551,8 +1552,8 @@ int main(int argc, char *argv[])
 {
 	int i, status = 0;
 	int interactive = 0;
-	char *oldpath = getenv("PATH");
 	struct libmnt_fs *fs;
+	const char *path = getenv("PATH");
 
 	setvbuf(stdout, NULL, _IONBF, BUFSIZ);
 	setvbuf(stderr, NULL, _IONBF, BUFSIZ);
@@ -1573,16 +1574,7 @@ int main(int argc, char *argv[])
 
 	load_fs_info();
 
-	/* Update our search path to include uncommon directories. */
-	if (oldpath) {
-		fsck_path = xmalloc (strlen (fsck_prefix_path) + 1 +
-				    strlen (oldpath) + 1);
-		strcpy (fsck_path, fsck_prefix_path);
-		strcat (fsck_path, ":");
-		strcat (fsck_path, oldpath);
-	} else {
-		fsck_path = xstrdup(fsck_prefix_path);
-	}
+	fsck_path = xstrdup(path && *path ? path : FSCK_DEFAULT_PATH);
 
 	if ((num_devices == 1) || (serialize))
 		interactive = 1;
-- 
2.1.0

Re: [PATCH 1/3] mkfs: drop hardcoded search path

[Karel Zak]

On Sun, Apr 12, 2015 at 05:06:51AM -0400, Mike Frysinger wrote:
>  disk-utils/mkfs.8 | 16 ++--------------
>  disk-utils/mkfs.c | 17 +----------------
>  2 files changed, 3 insertions(+), 30 deletions(-)

 Applied, thanks.

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