[PATCH 0/6] lib/path.c and lib/sys.c are friends (patches FOR TESTING ONLY)

[PATCH 0/6] lib/path.c and lib/sys.c are friends (patches FOR TESTING ONLY)

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Karel Zak wrote on github:

> BTW, see Documentation/TODO .
> 
> .. lib/path.c and lib/sys.c should be friends and they need our love :-) 
> 
> For lib/path.c should be probably nice to have handler, so we can use *at()
> functions rather than compose paths for each request.

Hi,

I've put this this trivial but non-dangerous solution together.
Using openat(), no cleanup yet, just to see how it looks.

See
 test_sysfs: usage: ./test_sysfs <sysroot> <devname>
                                   ^^^^^

and compare (maybe with stace)

 $ ln -s  /  /tmp/sysroot 
 $ ./test_sysfs /tmp/sysroot /dev/sda
 $ ./test_sysfs / /dev/sda
 $ ./test_sysfs "" /dev/sda

Note if sysroot is empty or not set at all, then the dirfd is ignored in all
openat() functions. Thus in the default case shouldn't be any risk that
the changes in path.c and sysfs.c can break anything. In theory we could
use path_open() and friends system wide instead if open(). For now I've
added some asserts to make sure that we are using path.h with absolute paths
only.

For the fun I've added option --sysroot to zramctl and lsblk. Don't know if it's
useful yet. The strace of lsblk looks like that we should use path_open()
functions at more places within libblkid.

So everybody is invited to create some "sysroot" tests like we have for lsmem and
lsblk already. Lets see whether if we can use this for something good.

cu,
Rudi

Ruediger Meier (6):
  path: use openat()
  path: add some standard functions
  path: never create path with prefix
  sysfs: use path.h
  zramctl: add --sysroot
  lsblk: add --sysroot

 include/path.h      |  15 ++++++
 lib/path.c          | 152 +++++++++++++++++++++++++++++++++++++++++++++-------
 lib/sysfs.c         |  53 +++++++++++-------
 misc-utils/lsblk.c  |  21 ++++++--
 sys-utils/lscpu.c   |   5 +-
 sys-utils/lsmem.c   |  11 ++--
 sys-utils/zramctl.c |  13 ++++-
 7 files changed, 214 insertions(+), 56 deletions(-)

-- 
1.8.5.6

[PATCH 1/6] path: use openat()

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

path_vcreate_with_prefix() is going to be removed.

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 lib/path.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 96 insertions(+), 15 deletions(-)

diff --git a/lib/path.c b/lib/path.c
index 79c1e7a..94181fe 100644
--- a/lib/path.c
+++ b/lib/path.c
@@ -26,18 +26,60 @@
 #include <stdio.h>
 #include <inttypes.h>
 #include <errno.h>
+#include <sys/stat.h>
 
 #include "all-io.h"
 #include "path.h"
 #include "nls.h"
 #include "c.h"
 
+static int rootfd = -1;
 static size_t prefixlen;
+static char pathbuf_prefix[PATH_MAX];
 static char pathbuf[PATH_MAX];
 
+/* It's a programming error if they try us to use with relative paths. */
+#define PATH_REQUIRE_ABSOLUTE(path) \
+	assert( !path || *path == '/' )
+
+static inline const char *
+path_relative_if_sysroot(const char *p)
+{
+	/* Convert to relative path if we have a prefix (sysroot). Otherwise
+	 * openat() functions would ignore the dirfd. */
+	if(p && rootfd >= 0)
+		while(*p && *p == '/') ++p;
+	return p;
+}
+
 static const char *
 path_vcreate(const char *path, va_list ap)
 {
+	const char * p = pathbuf;
+	int rc = vsnprintf(
+		pathbuf, sizeof(pathbuf), path, ap);
+
+	if (rc < 0)
+		goto err;
+	if ((size_t)rc >= sizeof(pathbuf)) {
+		errno = ENAMETOOLONG;
+		goto err;
+	}
+
+	PATH_REQUIRE_ABSOLUTE(p);
+	p = path_relative_if_sysroot(p);
+
+	return p;
+err:
+	/* Only for error messages when we could not construct a path. */
+	strcpy(pathbuf, "path");
+	return NULL;
+}
+
+static const char *
+path_vcreate_with_prefix(const char *path, va_list ap)
+{
+	strcpy(pathbuf, pathbuf_prefix);
 	int rc = vsnprintf(
 		pathbuf + prefixlen, sizeof(pathbuf) - prefixlen, path, ap);
 
@@ -55,30 +97,60 @@ path_get(const char *path, ...)
 {
 	const char *p;
 	va_list ap;
+	static char userbuf[sizeof(pathbuf)];
 
 	va_start(ap, path);
-	p = path_vcreate(path, ap);
+	p = path_vcreate_with_prefix(path, ap);
 	va_end(ap);
 
+	if (p) {
+		/* do not hand out our global buffer to the user to avoid overlapping
+		 * problems in case we get this string back from the user */
+		p = strcpy(userbuf, p);
+	}
+
 	return p;
 }
 
+/* Get the last used path. Only for error messages. */
+static const char *
+path_last_path(void)
+{
+	if (rootfd < 0) {
+		return pathbuf;
+	} else {
+		char prfx[] = "[sysroot]/"; /* we could also add the real prefix */
+		size_t n = sizeof(prfx) - 1 ;
+		memmove(pathbuf+n, pathbuf, sizeof(pathbuf)-n);
+		pathbuf[sizeof(pathbuf)-1] = '\0';
+		strncpy(pathbuf, prfx, n);
+		return pathbuf;
+	}
+}
+
 static FILE *
 path_vfopen(const char *mode, int exit_on_error, const char *path, va_list ap)
 {
+	int fd;
 	FILE *f;
 	const char *p = path_vcreate(path, ap);
 	if (!p)
 		goto err;
 
-	f = fopen(p, mode);
-	if (!f)
+	fd = openat(rootfd, p, O_RDONLY);
+	if (fd == -1)
+		goto err;
+
+	f = fdopen(fd, mode);
+	if (!f) {
+		close(fd);
 		goto err;
+	}
 
 	return f;
 err:
 	if (exit_on_error)
-		err(EXIT_FAILURE, _("cannot open %s"), p ? p : "path");
+		err(EXIT_FAILURE, _("cannot open %s"), path_last_path());
 	return NULL;
 }
 
@@ -90,13 +162,13 @@ path_vopen(int flags, const char *path, va_list ap)
 	if (!p)
 		goto err;
 
-	fd = open(p, flags);
+	fd = openat(rootfd, p, flags);
 	if (fd == -1)
 		goto err;
 
 	return fd;
 err:
-	err(EXIT_FAILURE, _("cannot open %s"), p ? p : "path");
+	err(EXIT_FAILURE, _("cannot open %s"), path_last_path());
 }
 
 FILE *
@@ -123,7 +195,7 @@ path_read_str(char *result, size_t len, const char *path, ...)
 	va_end(ap);
 
 	if (!fgets(result, len, fd))
-		err(EXIT_FAILURE, _("cannot read %s"), pathbuf);
+		err(EXIT_FAILURE, _("cannot read %s"), path_last_path());
 	fclose(fd);
 
 	len = strlen(result);
@@ -144,9 +216,9 @@ path_read_s32(const char *path, ...)
 
 	if (fscanf(fd, "%d", &result) != 1) {
 		if (ferror(fd))
-			err(EXIT_FAILURE, _("cannot read %s"), pathbuf);
+			err(EXIT_FAILURE, _("cannot read %s"), path_last_path());
 		else
-			errx(EXIT_FAILURE, _("parse error: %s"), pathbuf);
+			errx(EXIT_FAILURE, _("parse error: %s"), path_last_path());
 	}
 	fclose(fd);
 	return result;
@@ -165,9 +237,9 @@ path_read_u64(const char *path, ...)
 
 	if (fscanf(fd, "%"SCNu64, &result) != 1) {
 		if (ferror(fd))
-			err(EXIT_FAILURE, _("cannot read %s"), pathbuf);
+			err(EXIT_FAILURE, _("cannot read %s"), path_last_path());
 		else
-			errx(EXIT_FAILURE, _("parse error: %s"), pathbuf);
+			errx(EXIT_FAILURE, _("parse error: %s"), path_last_path());
 	}
 	fclose(fd);
 	return result;
@@ -197,7 +269,7 @@ path_exist(const char *path, ...)
 	p = path_vcreate(path, ap);
 	va_end(ap);
 
-	return p && access(p, F_OK) == 0;
+	return p && faccessat(rootfd, p, F_OK, 0) == 0;
 }
 
 #ifdef HAVE_CPU_SET_T
@@ -213,7 +285,7 @@ path_cpuparse(int maxcpus, int islist, const char *path, va_list ap)
 	fd = path_vfopen("r" UL_CLOEXECSTR, 1, path, ap);
 
 	if (!fgets(buf, len, fd))
-		err(EXIT_FAILURE, _("cannot read %s"), pathbuf);
+		err(EXIT_FAILURE, _("cannot read %s"), path_last_path());
 	fclose(fd);
 
 	len = strlen(buf);
@@ -265,12 +337,21 @@ path_set_prefix(const char *prefix)
 {
 	size_t len = strlen(prefix);
 
-	if (len >= sizeof(pathbuf) - 1) {
+	/* ignore trivial prefix */
+	if (len == 0)
+		return 0;
+
+	if (len >= sizeof(pathbuf_prefix) - 1) {
 		errno = ENAMETOOLONG;
 		return -1;
 	}
+
+	rootfd = open(prefix, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+	if (rootfd < 0)
+		return -1;
+
 	prefixlen = len;
-	strcpy(pathbuf, prefix);
+	strcpy(pathbuf_prefix, prefix);
 	return 0;
 }
 
-- 
1.8.5.6

[PATCH 2/6] path: add some standard functions

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 include/path.h | 15 +++++++++++++++
 lib/path.c     | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/include/path.h b/include/path.h
index ae36d7f..30c1133 100644
--- a/include/path.h
+++ b/include/path.h
@@ -3,6 +3,11 @@
 
 #include <stdio.h>
 #include <stdint.h>
+#include <dirent.h>
+
+/* Check your code: Absolute paths must be used with path_* functions! */
+#define PATH_REQUIRE_RELATIVE(path) \
+	assert( *path != '/' )
 
 /* Returns a pointer to a static buffer which may be destroyed by any later
 path_* function call. NULL means error and errno will be set. */
@@ -23,6 +28,16 @@ extern uint64_t path_read_u64(const char *path, ...)
 extern int path_exist(const char *path, ...)
 		      __attribute__ ((__format__ (__printf__, 1, 2)));
 
+extern int path_stat(const char *pathname, struct stat *buf);
+extern int path_open(const char *pathname, int flags);
+extern FILE *path_fopenP(const char *path, const char *mode);
+extern size_t path_readlink(const char *pathname, char *buf, size_t bufsiz);
+extern DIR *path_opendir(const char *dirname);
+extern int path_scandir(const char *dir, struct dirent ***namelist,
+	int (*sel)(const struct dirent *),
+	int (*compar)(const struct dirent **, const struct dirent **));
+
+
 #ifdef HAVE_CPU_SET_T
 # include "cpuset.h"
 
diff --git a/lib/path.c b/lib/path.c
index 94181fe..c286709 100644
--- a/lib/path.c
+++ b/lib/path.c
@@ -27,6 +27,7 @@
 #include <inttypes.h>
 #include <errno.h>
 #include <sys/stat.h>
+#include <dirent.h>
 
 #include "all-io.h"
 #include "path.h"
@@ -272,6 +273,56 @@ path_exist(const char *path, ...)
 	return p && faccessat(rootfd, p, F_OK, 0) == 0;
 }
 
+int
+path_stat(const char *p, struct stat *buf)
+{
+	PATH_REQUIRE_ABSOLUTE(p);
+	p = path_relative_if_sysroot(p);
+	return fstatat(rootfd, p, buf, 0);
+}
+
+int
+path_open(const char *p, int flags)
+{
+	PATH_REQUIRE_ABSOLUTE(p);
+	p = path_relative_if_sysroot(p);
+	return openat(rootfd, p, flags);
+}
+
+FILE*
+path_fopenP(const char *path, const char *mode)
+{
+	return path_fopen(mode, 0, path);
+}
+
+size_t
+path_readlink(const char *p, char *buf, size_t bufsiz)
+{
+	PATH_REQUIRE_ABSOLUTE(p);
+	p = path_relative_if_sysroot(p);
+	return readlinkat(rootfd, p, buf, bufsiz);
+}
+
+DIR *
+path_opendir(const char *dirname)
+{
+	int fd = path_open(dirname, O_RDONLY|O_CLOEXEC|O_DIRECTORY);
+	if (fd < 0) {
+		return NULL;
+	}
+	return fdopendir(fd);
+}
+
+int
+path_scandir(const char *p, struct dirent ***namelist,
+	int (*sel)(const struct dirent *),
+	int (*compar)(const struct dirent **, const struct dirent **))
+{
+	PATH_REQUIRE_ABSOLUTE(p);
+	p = path_relative_if_sysroot(p);
+	return scandirat(rootfd, p, namelist, sel, compar);
+}
+
 #ifdef HAVE_CPU_SET_T
 
 static cpu_set_t *
-- 
1.8.5.6

[PATCH 3/6] path: never create path with prefix

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 lib/path.c        | 20 ++------------------
 sys-utils/lscpu.c |  5 +----
 sys-utils/lsmem.c | 11 +++--------
 3 files changed, 6 insertions(+), 30 deletions(-)

diff --git a/lib/path.c b/lib/path.c
index c286709..ca18a7c 100644
--- a/lib/path.c
+++ b/lib/path.c
@@ -77,22 +77,6 @@ err:
 	return NULL;
 }
 
-static const char *
-path_vcreate_with_prefix(const char *path, va_list ap)
-{
-	strcpy(pathbuf, pathbuf_prefix);
-	int rc = vsnprintf(
-		pathbuf + prefixlen, sizeof(pathbuf) - prefixlen, path, ap);
-
-	if (rc < 0)
-		return NULL;
-	if ((size_t)rc >= sizeof(pathbuf)) {
-		errno = ENAMETOOLONG;
-		return NULL;
-	}
-	return pathbuf;
-}
-
 const char *
 path_get(const char *path, ...)
 {
@@ -101,13 +85,13 @@ path_get(const char *path, ...)
 	static char userbuf[sizeof(pathbuf)];
 
 	va_start(ap, path);
-	p = path_vcreate_with_prefix(path, ap);
+	p = path_vcreate(path, ap);
 	va_end(ap);
 
 	if (p) {
 		/* do not hand out our global buffer to the user to avoid overlapping
 		 * problems in case we get this string back from the user */
-		p = strcpy(userbuf, p);
+		p = strcpy(userbuf, pathbuf);
 	}
 
 	return p;
diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
index 8b4fe0b..fa901fe 100644
--- a/sys-utils/lscpu.c
+++ b/sys-utils/lscpu.c
@@ -1430,14 +1430,11 @@ read_nodes(struct lscpu_desc *desc)
 	int i = 0;
 	DIR *dir;
 	struct dirent *d;
-	const char *path;
 
 	desc->nnodes = 0;
 
 	/* number of NUMA node */
-	if (!(path = path_get(_PATH_SYS_NODE)))
-		return;
-	if (!(dir = opendir(path)))
+	if (!(dir = path_opendir(_PATH_SYS_NODE)))
 		return;
 	while ((d = readdir(dir))) {
 		if (is_node_dirent(d))
diff --git a/sys-utils/lsmem.c b/sys-utils/lsmem.c
index aeffd29..b8d827d 100644
--- a/sys-utils/lsmem.c
+++ b/sys-utils/lsmem.c
@@ -253,7 +253,7 @@ static int memory_block_get_node(char *name)
 	int node;
 
 	path = path_get(_PATH_SYS_MEMORY"/%s", name);
-	if (!path || !(dir= opendir(path)))
+	if (!path || !(dir = path_opendir(path)))
 		err(EXIT_FAILURE, _("Failed to open %s"), path ? path : name);
 
 	node = -1;
@@ -347,16 +347,11 @@ static int memory_block_filter(const struct dirent *de)
 
 static void read_basic_info(struct lsmem *lsmem)
 {
-	const char *dir;
-
 	if (!path_exist(_PATH_SYS_MEMORY_BLOCK_SIZE))
 		errx(EXIT_FAILURE, _("This system does not support memory blocks"));
 
-	dir = path_get(_PATH_SYS_MEMORY);
-	if (!dir)
-		err(EXIT_FAILURE, _("Failed to read %s"), _PATH_SYS_MEMORY);
-
-	lsmem->ndirs = scandir(dir, &lsmem->dirs, memory_block_filter, versionsort);
+	lsmem->ndirs = path_scandir(_PATH_SYS_MEMORY,
+			&lsmem->dirs, memory_block_filter, versionsort);
 	if (lsmem->ndirs <= 0)
 		err(EXIT_FAILURE, _("Failed to read %s"), _PATH_SYS_MEMORY);
 
-- 
1.8.5.6

[PATCH 4/6] sysfs: use path.h

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 lib/sysfs.c | 53 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/lib/sysfs.c b/lib/sysfs.c
index 68b43aa..8b9a6aa 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -13,6 +13,7 @@
 #include "c.h"
 #include "pathnames.h"
 #include "sysfs.h"
+#include "path.h"
 #include "fileutils.h"
 #include "all-io.h"
 
@@ -38,7 +39,7 @@ int sysfs_devno_has_attribute(dev_t devno, const char *attr)
 
 	if (!sysfs_devno_attribute_path(devno, path, sizeof(path), attr))
 		return 0;
-	if (stat(path, &info) == 0)
+	if (path_stat(path, &info) == 0)
 		return 1;
 	return 0;
 }
@@ -59,7 +60,7 @@ dev_t sysfs_devname_to_devno(const char *name, const char *parent)
 		 */
 		struct stat st;
 
-		if (stat(name, &st) == 0)
+		if (path_stat(name, &st) == 0)
 			dev = st.st_rdev;
 		else
 			name += 5;	/* unaccesible, or not node in /dev */
@@ -114,7 +115,7 @@ dev_t sysfs_devname_to_devno(const char *name, const char *parent)
 		FILE *f;
 		int maj = 0, min = 0;
 
-		f = fopen(path, "r" UL_CLOEXECSTR);
+		f = path_fopenP(path, "r" UL_CLOEXECSTR);
 		if (!f)
 			return 0;
 
@@ -155,7 +156,7 @@ char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz)
 	memmove(buf + 5, name, sz + 1);
 	memcpy(buf, "/dev/", 5);
 
-	if (!stat(buf, &st) && S_ISBLK(st.st_mode) && st.st_rdev == devno)
+	if (!path_stat(buf, &st) && S_ISBLK(st.st_mode) && st.st_rdev == devno)
 		return buf;
 
 	return NULL;
@@ -172,7 +173,7 @@ int sysfs_init(struct sysfs_cxt *cxt, dev_t devno, struct sysfs_cxt *parent)
 	if (!sysfs_devno_path(devno, path, sizeof(path)))
 		goto err;
 
-	fd = open(path, O_RDONLY|O_CLOEXEC);
+	fd = path_open(path, O_RDONLY|O_CLOEXEC);
 	if (fd < 0)
 		goto err;
 	cxt->dir_fd = fd;
@@ -205,7 +206,10 @@ void sysfs_deinit(struct sysfs_cxt *cxt)
 
 int sysfs_stat(struct sysfs_cxt *cxt, const char *attr, struct stat *st)
 {
-	int rc = fstatat(cxt->dir_fd, attr, st, 0);
+	int rc;
+
+	PATH_REQUIRE_RELATIVE(attr);
+	rc = fstatat(cxt->dir_fd, attr, st, 0);
 
 	if (rc != 0 && errno == ENOENT &&
 	    strncmp(attr, "queue/", 6) == 0 && cxt->parent) {
@@ -227,7 +231,10 @@ int sysfs_has_attribute(struct sysfs_cxt *cxt, const char *attr)
 
 static int sysfs_open(struct sysfs_cxt *cxt, const char *attr, int flags)
 {
-	int fd = openat(cxt->dir_fd, attr, flags);
+	int fd;
+
+	PATH_REQUIRE_RELATIVE(attr);
+	fd = openat(cxt->dir_fd, attr, flags);
 
 	if (fd == -1 && errno == ENOENT &&
 	    strncmp(attr, "queue/", 6) == 0 && cxt->parent) {
@@ -246,11 +253,12 @@ ssize_t sysfs_readlink(struct sysfs_cxt *cxt, const char *attr,
 	if (!cxt->dir_path)
 		return -1;
 
-	if (attr)
+	if (attr) {
+		PATH_REQUIRE_RELATIVE(attr);
 		return readlinkat(cxt->dir_fd, attr, buf, bufsiz);
-
+	}
 	/* read /sys/dev/block/<maj:min> link */
-	return readlink(cxt->dir_path, buf, bufsiz);
+	return path_readlink(cxt->dir_path, buf, bufsiz);
 }
 
 DIR *sysfs_opendir(struct sysfs_cxt *cxt, const char *attr)
@@ -342,6 +350,7 @@ int sysfs_is_partition_dirent(DIR *dir, struct dirent *d, const char *parent_nam
 	/* Cannot use /partition file, not supported on old sysfs */
 	snprintf(path, sizeof(path), "%s/start", d->d_name);
 
+	PATH_REQUIRE_RELATIVE(path);
 	return faccessat(dirfd(dir), path, R_OK, 0) == 0;
 }
 
@@ -601,7 +610,7 @@ static char *get_subsystem(char *chain, char *buf, size_t bufsz)
 		memcpy(chain + len, SUBSYSTEM_LINKNAME, sizeof(SUBSYSTEM_LINKNAME));
 
 		/* try if subsystem symlink exists */
-		sz = readlink(chain, buf, bufsz - 1);
+		sz = path_readlink(chain, buf, bufsz - 1);
 
 		/* remove last subsystem from chain */
 		chain[len] = '\0';
@@ -946,7 +955,7 @@ char *sysfs_scsi_host_strdup_attribute(struct sysfs_cxt *cxt,
 	    !sysfs_scsi_host_attribute_path(cxt, type, buf, sizeof(buf), attr))
 		return NULL;
 
-	if (!(f = fopen(buf, "r" UL_CLOEXECSTR)))
+	if (!(f = path_fopenP(buf, "r" UL_CLOEXECSTR)))
                 return NULL;
 
 	rc = fscanf(f, "%1023[^\n]", buf);
@@ -964,7 +973,7 @@ int sysfs_scsi_host_is(struct sysfs_cxt *cxt, const char *type)
 				buf, sizeof(buf), NULL))
 		return 0;
 
-	return stat(buf, &st) == 0 && S_ISDIR(st.st_mode);
+	return path_stat(buf, &st) == 0 && S_ISDIR(st.st_mode);
 }
 
 static char *sysfs_scsi_attribute_path(struct sysfs_cxt *cxt,
@@ -992,7 +1001,7 @@ int sysfs_scsi_has_attribute(struct sysfs_cxt *cxt, const char *attr)
 	if (!sysfs_scsi_attribute_path(cxt, path, sizeof(path), attr))
 		return 0;
 
-	return stat(path, &st) == 0;
+	return path_stat(path, &st) == 0;
 }
 
 int sysfs_scsi_path_contains(struct sysfs_cxt *cxt, const char *pattern)
@@ -1004,10 +1013,10 @@ int sysfs_scsi_path_contains(struct sysfs_cxt *cxt, const char *pattern)
 	if (!sysfs_scsi_attribute_path(cxt, path, sizeof(path), NULL))
 		return 0;
 
-	if (stat(path, &st) != 0)
+	if (path_stat(path, &st) != 0)
 		return 0;
 
-	len = readlink(path, linkc, sizeof(linkc) - 1);
+	len = path_readlink(path, linkc, sizeof(linkc) - 1);
 	if (len < 0)
 		return 0;
 
@@ -1024,6 +1033,7 @@ int main(int argc, char *argv[])
 {
 	struct sysfs_cxt cxt = UL_SYSFSCXT_EMPTY;
 	char *devname;
+	char *sysroot;
 	dev_t devno, disk_devno;
 	char path[PATH_MAX], *sub, *chain;
 	char diskname[32];
@@ -1031,10 +1041,15 @@ int main(int argc, char *argv[])
 	uint64_t u64;
 	ssize_t len;
 
-	if (argc != 2)
-		errx(EXIT_FAILURE, "usage: %s <devname>", argv[0]);
+	if (argc != 3)
+		errx(EXIT_FAILURE, "usage: %s <sysroot> <devname>", argv[0]);
+
+	sysroot = argv[1];
+	devname = argv[2];
+
+	if(path_set_prefix(sysroot))
+		err(EXIT_FAILURE, "invalid argument to %s", "--sysroot");
 
-	devname = argv[1];
 	devno = sysfs_devname_to_devno(devname, NULL);
 
 	if (!devno)
-- 
1.8.5.6

[PATCH 5/6] zramctl: add --sysroot

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 sys-utils/zramctl.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/sys-utils/zramctl.c b/sys-utils/zramctl.c
index aee28cc..c6224b3 100644
--- a/sys-utils/zramctl.c
+++ b/sys-utils/zramctl.c
@@ -278,7 +278,7 @@ static int zram_used(struct zram *z)
 static int zram_has_control(struct zram *z)
 {
 	if (!z->control_probed) {
-		z->has_control = access(_PATH_SYS_CLASS "/zram-control/", F_OK) == 0 ? 1 : 0;
+		z->has_control = path_exist(_PATH_SYS_CLASS "/zram-control/") == 0 ? 1 : 0;
 		z->control_probed = 1;
 		DBG(fprintf(stderr, "zram-control: %s", z->has_control ? "yes" : "no"));
 	}
@@ -569,7 +569,10 @@ int main(int argc, char **argv)
 	int rc = 0, c, find = 0, act = A_NONE;
 	struct zram *zram = NULL;
 
-	enum { OPT_RAW = CHAR_MAX + 1 };
+	enum {
+		OPT_RAW = CHAR_MAX + 1,
+		OPT_SYSROOT,
+	};
 
 	static const struct option longopts[] = {
 		{ "algorithm", required_argument, NULL, 'a' },
@@ -583,6 +586,7 @@ int main(int argc, char **argv)
 		{ "size",      required_argument, NULL, 's' },
 		{ "streams",   required_argument, NULL, 't' },
 		{ "version",   no_argument, NULL, 'V' },
+		{ "sysroot",   required_argument, NULL, OPT_SYSROOT },
 		{ NULL, 0, NULL, 0 }
 	};
 
@@ -641,6 +645,11 @@ int main(int argc, char **argv)
 		case 'V':
 			printf(UTIL_LINUX_VERSION);
 			return EXIT_SUCCESS;
+		case OPT_SYSROOT:
+			if(path_set_prefix(optarg))
+				err(EXIT_FAILURE, _("invalid argument to %s"), "--sysroot");
+			/* TODO: mod->system = SYSTEM_SNAPSHOT; */
+			break;
 		case 'h':
 			usage();
 		default:
-- 
1.8.5.6

[PATCH 6/6] lsblk: add --sysroot

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 misc-utils/lsblk.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/misc-utils/lsblk.c b/misc-utils/lsblk.c
index 9d3460b..c7f5b0e 100644
--- a/misc-utils/lsblk.c
+++ b/misc-utils/lsblk.c
@@ -57,6 +57,7 @@
 #include "xalloc.h"
 #include "strutils.h"
 #include "sysfs.h"
+#include "path.h"
 #include "closestream.h"
 #include "mangle.h"
 #include "optutils.h"
@@ -640,7 +641,7 @@ static int is_readonly_device(struct blkdev_cxt *cxt)
 		return ro;
 
 	/* fallback if "ro" attribute does not exist */
-	fd = open(cxt->filename, O_RDONLY);
+	fd = path_open(cxt->filename, O_RDONLY);
 	if (fd != -1) {
 		if (ioctl(fd, BLKROGET, &ro) != 0)
 			ro = 0;
@@ -892,7 +893,7 @@ static void set_scols_data(struct blkdev_cxt *cxt, int col, int id, struct libsc
 
 	if (!cxt->st.st_rdev && (id == COL_OWNER || id == COL_GROUP ||
 				 id == COL_MODE))
-		st_rc = stat(cxt->filename, &cxt->st);
+		st_rc = path_stat(cxt->filename, &cxt->st);
 
 	if (lsblk->sort_id == id)
 		sort = 1;
@@ -1437,7 +1438,7 @@ static int iterate_block_devices(void)
 	struct dirent *d;
 	struct blkdev_cxt cxt = { NULL };
 
-	if (!(dir = opendir(_PATH_SYS_BLOCK)))
+	if (!(dir = path_opendir(_PATH_SYS_BLOCK)))
 		return -errno;
 
 	DBG(DEV, ul_debug("iterate on " _PATH_SYS_BLOCK));
@@ -1477,7 +1478,7 @@ static char *devno_to_sysfs_name(dev_t devno, char *devname, char *buf, size_t b
 		return NULL;
 	}
 
-	len = readlink(path, buf, buf_size - 1);
+	len = path_readlink(path, buf, buf_size - 1);
 	if (len < 0) {
 		warn(_("%s: failed to read link"), path);
 		return NULL;
@@ -1495,7 +1496,7 @@ static int process_one_device(char *devname)
 	dev_t disk = 0;
 	int real_part = 0, rc = -EINVAL;
 
-	if (stat(devname, &st) || !S_ISBLK(st.st_mode)) {
+	if (path_stat(devname, &st) || !S_ISBLK(st.st_mode)) {
 		warnx(_("%s: not a block device"), devname);
 		goto leave;
 	}
@@ -1685,6 +1686,10 @@ int main(int argc, char *argv[])
 	size_t i;
 	int force_tree = 0;
 
+	enum {
+		OPT_SYSROOT = CHAR_MAX + 1,
+	};
+
 	static const struct option longopts[] = {
 		{ "all",	no_argument,       NULL, 'a' },
 		{ "bytes",      no_argument,       NULL, 'b' },
@@ -1711,6 +1716,7 @@ int main(int argc, char *argv[])
 		{ "sort",	required_argument, NULL, 'x' },
 		{ "tree",       no_argument,       NULL, 'T' },
 		{ "version",    no_argument,       NULL, 'V' },
+		{ "sysroot",    required_argument, NULL, OPT_SYSROOT },
 		{ NULL, 0, NULL, 0 },
 	};
 
@@ -1848,6 +1854,11 @@ int main(int argc, char *argv[])
 		case 'V':
 			printf(UTIL_LINUX_VERSION);
 			return EXIT_SUCCESS;
+		case OPT_SYSROOT:
+			if(path_set_prefix(optarg))
+				err(EXIT_FAILURE, _("invalid argument to %s"), "--sysroot");
+			/* TODO: mod->system = SYSTEM_SNAPSHOT; */
+			break;
 		case 'x':
 			lsblk->flags &= ~LSBLK_TREE; /* disable the default */
 			lsblk->sort_id = column_name_to_id(optarg, strlen(optarg));
-- 
1.8.5.6

Re: [PATCH 0/6] lib/path.c and lib/sys.c are friends (patches FOR TESTING ONLY)

[Ruediger Meier]

BTW this is also available via github.

branch "path"
https://github.com/rudimeier/util-linux

On Monday 03 July 2017, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
>
> Karel Zak wrote on github:
> > BTW, see Documentation/TODO .
> >
> > .. lib/path.c and lib/sys.c should be friends and they need our
> > love :-)
> >
> > For lib/path.c should be probably nice to have handler, so we can
> > use *at() functions rather than compose paths for each request.
>
> Hi,
>
> I've put this this trivial but non-dangerous solution together.
> Using openat(), no cleanup yet, just to see how it looks.
>
> See
>  test_sysfs: usage: ./test_sysfs <sysroot> <devname>
>                                    ^^^^^
>
> and compare (maybe with stace)
>
>  $ ln -s  /  /tmp/sysroot
>  $ ./test_sysfs /tmp/sysroot /dev/sda
>  $ ./test_sysfs / /dev/sda
>  $ ./test_sysfs "" /dev/sda
>
> Note if sysroot is empty or not set at all, then the dirfd is ignored
> in all openat() functions. Thus in the default case shouldn't be any
> risk that the changes in path.c and sysfs.c can break anything. In
> theory we could use path_open() and friends system wide instead if
> open(). For now I've added some asserts to make sure that we are
> using path.h with absolute paths only.
>
> For the fun I've added option --sysroot to zramctl and lsblk. Don't
> know if it's useful yet. The strace of lsblk looks like that we
> should use path_open() functions at more places within libblkid.
>
> So everybody is invited to create some "sysroot" tests like we have
> for lsmem and lsblk already. Lets see whether if we can use this for
> something good.
>
> cu,
> Rudi
>
> Ruediger Meier (6):
>   path: use openat()
>   path: add some standard functions
>   path: never create path with prefix
>   sysfs: use path.h
>   zramctl: add --sysroot
>   lsblk: add --sysroot
>
>  include/path.h      |  15 ++++++
>  lib/path.c          | 152
> +++++++++++++++++++++++++++++++++++++++++++++------- lib/sysfs.c     
>    |  53 +++++++++++-------
>  misc-utils/lsblk.c  |  21 ++++++--
>  sys-utils/lscpu.c   |   5 +-
>  sys-utils/lsmem.c   |  11 ++--
>  sys-utils/zramctl.c |  13 ++++-
>  7 files changed, 214 insertions(+), 56 deletions(-)

Re: [PATCH 2/6] path: add some standard functions

[Karel Zak]

On Mon, Jul 03, 2017 at 01:52:57PM +0200, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
> 
> Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> ---
>  include/path.h | 15 +++++++++++++++
>  lib/path.c     | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/include/path.h b/include/path.h
> index ae36d7f..30c1133 100644
> --- a/include/path.h
> +++ b/include/path.h
> @@ -3,6 +3,11 @@
>  
>  #include <stdio.h>
>  #include <stdint.h>
> +#include <dirent.h>
> +
> +/* Check your code: Absolute paths must be used with path_* functions! */
> +#define PATH_REQUIRE_RELATIVE(path) \
> +	assert( *path != '/' )

 #define assert_abs_path

or something else with "assert" :-)

    Karel

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

Re: [PATCH 1/6] path: use openat()

[Karel Zak]

On Mon, Jul 03, 2017 at 01:52:56PM +0200, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
> 
> path_vcreate_with_prefix() is going to be removed.
> 
> Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> ---
>  lib/path.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 96 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/path.c b/lib/path.c
> index 79c1e7a..94181fe 100644
> --- a/lib/path.c
> +++ b/lib/path.c
> @@ -26,18 +26,60 @@
>  #include <stdio.h>
>  #include <inttypes.h>
>  #include <errno.h>
> +#include <sys/stat.h>
>  
>  #include "all-io.h"
>  #include "path.h"
>  #include "nls.h"
>  #include "c.h"
>  
> +static int rootfd = -1;
>  static size_t prefixlen;
> +static char pathbuf_prefix[PATH_MAX];
>  static char pathbuf[PATH_MAX];

We use sysfs.c in shared libs, it means that global variables are
unacceptable.

It would be better to have "struct path_cxt" and use it within "struct
sysfs_cxt". For tools where we don't use "struct sysfs_cxt" (e.g.
lscpu) we can directly use "struct path_cxt".

I guess it should be possible to replace sysfs_cxt->dir_fd with
"struct path_cxt" where it points to $SYSROOT/sysfs/block/<device>.

    Karel

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

Re: [PATCH 1/6] path: use openat()

[Karel Zak]

On Mon, Jul 03, 2017 at 04:39:49PM +0200, Karel Zak wrote:
> On Mon, Jul 03, 2017 at 01:52:56PM +0200, Ruediger Meier wrote:
> > From: Ruediger Meier <ruediger.meier@ga-group.nl>
> > 
> > path_vcreate_with_prefix() is going to be removed.
> > 
> > Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> > ---
> >  lib/path.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 96 insertions(+), 15 deletions(-)
> > 
> > diff --git a/lib/path.c b/lib/path.c
> > index 79c1e7a..94181fe 100644
> > --- a/lib/path.c
> > +++ b/lib/path.c
> > @@ -26,18 +26,60 @@
> >  #include <stdio.h>
> >  #include <inttypes.h>
> >  #include <errno.h>
> > +#include <sys/stat.h>
> >  
> >  #include "all-io.h"
> >  #include "path.h"
> >  #include "nls.h"
> >  #include "c.h"
> >  
> > +static int rootfd = -1;
> >  static size_t prefixlen;
> > +static char pathbuf_prefix[PATH_MAX];
> >  static char pathbuf[PATH_MAX];
> 
> We use sysfs.c in shared libs, it means that global variables are
> unacceptable.
> 
> It would be better to have "struct path_cxt" and use it within "struct
> sysfs_cxt". For tools where we don't use "struct sysfs_cxt" (e.g.
> lscpu) we can directly use "struct path_cxt".
> 
> I guess it should be possible to replace sysfs_cxt->dir_fd with
> "struct path_cxt" where it points to $SYSROOT/sysfs/block/<device>.

And if we want to be real perfectionists than in sysfs.c we should
differentiate between block functions ($SYSROOT/sysfs/block/) and
another targets ($SYSROOT/sys/devices/system/cpu, ...etc).

Examples:

path.c only code:

 uint64_t x;
 struct path_cxt *path = path_new_context();

 path_set_system_root(path, argv[1]);

 path_read_s32(path, _PATH_SYS_CPU "/kernel_max", &x);


sysfs based code:

 uint64_t x;
 struct path_cxt *path = path_new_context();

 path_set_system_root(path, argv[1]);

 sysfs = sysfs_new_context(path);
 sysfs_assign_block_device(sysfs, devno, NULL);
 
 sysfs_read_u64(sysfs, "queue/minimum_io_size", &x);


Do you see more light in the proposal now? :-)

    Karel

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

Re: [PATCH 1/6] path: use openat()

[Ruediger Meier]

On Monday 03 July 2017, Karel Zak wrote:
> On Mon, Jul 03, 2017 at 04:39:49PM +0200, Karel Zak wrote:
> > On Mon, Jul 03, 2017 at 01:52:56PM +0200, Ruediger Meier wrote:
> > > From: Ruediger Meier <ruediger.meier@ga-group.nl>
> > >
> > > path_vcreate_with_prefix() is going to be removed.
> > >
> > > Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> > > ---
> > >  lib/path.c | 111
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1
> > > file changed, 96 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/lib/path.c b/lib/path.c
> > > index 79c1e7a..94181fe 100644
> > > --- a/lib/path.c
> > > +++ b/lib/path.c
> > > @@ -26,18 +26,60 @@
> > >  #include <stdio.h>
> > >  #include <inttypes.h>
> > >  #include <errno.h>
> > > +#include <sys/stat.h>
> > >
> > >  #include "all-io.h"
> > >  #include "path.h"
> > >  #include "nls.h"
> > >  #include "c.h"
> > >
> > > +static int rootfd = -1;
> > >  static size_t prefixlen;
> > > +static char pathbuf_prefix[PATH_MAX];
> > >  static char pathbuf[PATH_MAX];
> >
> > We use sysfs.c in shared libs, it means that global variables are
> > unacceptable.

Yes I understand. This global way was just for first tests without 
changing much code. To be honest, while I was very happy first about 
quick results, I am now a bit unsure whether this sysroot feature will 
be much useful at all.

See, even lscpu --sysroot does not work fully transparently. We have to 
ignore "Architecture". So maybe we would also need a fake-syscall 
interface for many cases!? Also we are sometimes writing to /sys and 
probably expect some events.

I wonder if it wouldn't be easier to construct chroot or container 
environments with whatever /sys, /proc, /etc. to test certain things.

So I thought we could use my quick-and-dirty approach to implement some 
more --sysroot tests just to see if we will get something useful at 
all.

cu,
Rudi

> > It would be better to have "struct path_cxt" and use it within
> > "struct sysfs_cxt". For tools where we don't use "struct sysfs_cxt"
> > (e.g. lscpu) we can directly use "struct path_cxt".
> >
> > I guess it should be possible to replace sysfs_cxt->dir_fd with
> > "struct path_cxt" where it points to $SYSROOT/sysfs/block/<device>.

> And if we want to be real perfectionists than in sysfs.c we should
> differentiate between block functions ($SYSROOT/sysfs/block/) and
> another targets ($SYSROOT/sys/devices/system/cpu, ...etc).
>
> Examples:
>
> path.c only code:
>
>  uint64_t x;
>  struct path_cxt *path = path_new_context();
>
>  path_set_system_root(path, argv[1]);
>
>  path_read_s32(path, _PATH_SYS_CPU "/kernel_max", &x);
>
>
> sysfs based code:
>
>  uint64_t x;
>  struct path_cxt *path = path_new_context();
>
>  path_set_system_root(path, argv[1]);
>
>  sysfs = sysfs_new_context(path);
>  sysfs_assign_block_device(sysfs, devno, NULL);
>
>  sysfs_read_u64(sysfs, "queue/minimum_io_size", &x);
>
>
> Do you see more light in the proposal now? :-)

Re: [PATCH 1/6] path: use openat()

[Karel Zak]

On Mon, Jul 03, 2017 at 06:11:56PM +0200, Ruediger Meier wrote:
> Yes I understand. This global way was just for first tests without 
> changing much code. To be honest, while I was very happy first about 
> quick results, I am now a bit unsure whether this sysroot feature will 
> be much useful at all.
> 
> See, even lscpu --sysroot does not work fully transparently. We have to 
> ignore "Architecture". So maybe we would also need a fake-syscall 
> interface for many cases!? Also we are sometimes writing to /sys and 
> probably expect some events.

well, zramctl writes to /sys, but another utils just read.

> I wonder if it wouldn't be easier to construct chroot or container 
> environments with whatever /sys, /proc, /etc. to test certain things.

Yes, chroot would be good enough for the tests.

The path functions are not only about --sysroot, but it also provides
interface for things like "read file content as utnt64, strdup(), etc.

> So I thought we could use my quick-and-dirty approach to implement some 
> more --sysroot tests just to see if we will get something useful at 
> all.

I do not see a problem to kill --sysroot and use chroot, but we still
need openat and another things.

    Karel

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