[hackers] [PATCH 2/2] libutil/recurse: Use a single path buffer, and directory fd

From: Michael Forney <mforney_AT_mforney.org>
Date: Sun, 22 Dec 2019 17:03:59 -0800

This way, we don't use PATH_MAX bytes on the stack per path component,
and don't have to keep copying the complete path around.
---
 chgrp.c           | 11 +++++------
 chmod.c           | 11 ++++++-----
 chown.c           | 10 +++++-----
 du.c              | 13 +++++++------
 fs.h              |  9 ++++++---
 libutil/recurse.c | 45 ++++++++++++++++++++++++++-------------------
 libutil/rm.c      | 13 +++++++------
 mv.c              |  3 ++-
 rm.c              |  4 +++-
 tar.c             | 10 +++++-----
 10 files changed, 72 insertions(+), 57 deletions(-)
diff --git a/chgrp.c b/chgrp.c
index 04b2dd0..d336c88 100644
--- a/chgrp.c
+++ b/chgrp.c
_AT_@ -14,18 +14,17 @@ static gid_t gid = -1;
 static int   ret = 0;
 
 static void
-chgrp(const char *path, struct stat *st, void *data, struct recursor *r)
+chgrp(int dirfd, const char *name, struct stat *st, void *data, struct recursor *r)
 {
 	int flags = 0;
 
 	if ((r->maxdepth == 0 && r->follow == 'P') || (r->follow == 'H' && r->depth) || (hflag && !(r->depth)))
 		flags |= AT_SYMLINK_NOFOLLOW;
-
-	if (fchownat(AT_FDCWD, path, -1, gid, flags) < 0) {
-		weprintf("chown %s:", path);
+	if (fchownat(dirfd, name, -1, gid, flags) < 0) {
+		weprintf("chown %s:", r->path);
 		ret = 1;
 	} else if (S_ISDIR(st->st_mode)) {
-		recurse(path, NULL, r);
+		recurse(dirfd, name, NULL, r);
 	}
 }
 
_AT_@ -71,7 +70,7 @@ main(int argc, char *argv[])
 	}
 
 	for (argc--, argv++; *argv; argc--, argv++)
-		recurse(*argv, NULL, &r);
+		recurse(AT_FDCWD, *argv, NULL, &r);
 
 	return ret || recurse_status;
 }
diff --git a/chmod.c b/chmod.c
index 2a0085d..3d63791 100644
--- a/chmod.c
+++ b/chmod.c
_AT_@ -1,4 +1,5 @@
 /* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
 #include <sys/stat.h>
 
 #include "fs.h"
_AT_@ -9,16 +10,16 @@ static mode_t mask    = 0;
 static int    ret     = 0;
 
 static void
-chmodr(const char *path, struct stat *st, void *data, struct recursor *r)
+chmodr(int dirfd, const char *name, struct stat *st, void *data, struct recursor *r)
 {
 	mode_t m;
 
 	m = parsemode(modestr, st->st_mode & ~S_IFMT, mask);
-	if (chmod(path, m) < 0) {
-		weprintf("chmod %s:", path);
+	if (fchmodat(dirfd, name, m, 0) < 0) {
+		weprintf("chmod %s:", r->path);
 		ret = 1;
 	} else if (S_ISDIR(st->st_mode)) {
-		recurse(path, NULL, r);
+		recurse(dirfd, name, NULL, r);
 	}
 }
 
_AT_@ -76,7 +77,7 @@ done:
 		usage();
 
 	for (--argc, ++argv; *argv; argc--, argv++)
-		recurse(*argv, NULL, &r);
+		recurse(AT_FDCWD, *argv, NULL, &r);
 
 	return ret || recurse_status;
 }
diff --git a/chown.c b/chown.c
index dcd4914..3aaedef 100644
--- a/chown.c
+++ b/chown.c
_AT_@ -17,18 +17,18 @@ static gid_t gid = -1;
 static int   ret = 0;
 
 static void
-chownpwgr(const char *path, struct stat *st, void *data, struct recursor *r)
+chownpwgr(int dirfd, const char *name, struct stat *st, void *data, struct recursor *r)
 {
 	int flags = 0;
 
 	if ((r->maxdepth == 0 && r->follow == 'P') || (r->follow == 'H' && r->depth) || (hflag && !(r->depth)))
 		flags |= AT_SYMLINK_NOFOLLOW;
 
-	if (fchownat(AT_FDCWD, path, uid, gid, flags) < 0) {
-		weprintf("chown %s:", path);
+	if (fchownat(dirfd, name, uid, gid, flags) < 0) {
+		weprintf("chown %s:", r->path);
 		ret = 1;
 	} else if (S_ISDIR(st->st_mode)) {
-		recurse(path, NULL, r);
+		recurse(dirfd, name, NULL, r);
 	}
 }
 
_AT_@ -99,7 +99,7 @@ main(int argc, char *argv[])
 		usage();
 
 	for (argc--, argv++; *argv; argc--, argv++)
-		recurse(*argv, NULL, &r);
+		recurse(AT_FDCWD, *argv, NULL, &r);
 
 	return ret || recurse_status;
 }
diff --git a/du.c b/du.c
index 4ceac66..fa6a8d6 100644
--- a/du.c
+++ b/du.c
_AT_@ -3,6 +3,7 @@
 #include <sys/types.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <stdint.h>
 #include <stdlib.h>
_AT_@ -35,19 +36,19 @@ nblks(blkcnt_t blocks)
 }
 
 static void
-du(const char *path, struct stat *st, void *data, struct recursor *r)
+du(int dirfd, const char *path, struct stat *st, void *data, struct recursor *r)
 {
 	off_t *total = data, subtotal;
 
 	subtotal = nblks(st->st_blocks);
 	if (S_ISDIR(st->st_mode))
-		recurse(path, &subtotal, r);
+		recurse(dirfd, path, &subtotal, r);
 	*total += subtotal;
 
 	if (!r->depth)
-		printpath(*total, path);
+		printpath(*total, r->path);
 	else if (!sflag && r->depth <= maxdepth && (S_ISDIR(st->st_mode) || aflag))
-		printpath(subtotal, path);
+		printpath(subtotal, r->path);
 }
 
 static void
_AT_@ -104,11 +105,11 @@ main(int argc, char *argv[])
 		blksize = 1024;
 
 	if (!argc) {
-		recurse(".", &n, &r);
+		recurse(AT_FDCWD, ".", &n, &r);
 	} else {
 		for (; *argv; argc--, argv++) {
 			n = 0;
-			recurse(*argv, &n, &r);
+			recurse(AT_FDCWD, *argv, &n, &r);
 		}
 	}
 
diff --git a/fs.h b/fs.h
index 15ae5f4..00ecd3b 100644
--- a/fs.h
+++ b/fs.h
_AT_@ -1,4 +1,5 @@
 /* See LICENSE file for copyright and license details. */
+#include <limits.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 
_AT_@ -9,7 +10,9 @@ struct history {
 };
 
 struct recursor {
-	void (*fn)(const char *, struct stat *st, void *, struct recursor *);
+	void (*fn)(int, const char *, struct stat *, void *, struct recursor *);
+	char path[PATH_MAX];
+	size_t pathlen;
 	struct history *hist;
 	int depth;
 	int maxdepth;
_AT_@ -37,7 +40,7 @@ extern int rm_status;
 
 extern int recurse_status;
 
-void recurse(const char *, void *, struct recursor *);
+void recurse(int, const char *, void *, struct recursor *);
 
 int cp(const char *, const char *, int);
-void rm(const char *, struct stat *st, void *, struct recursor *);
+void rm(int, const char *, struct stat *st, void *, struct recursor *);
diff --git a/libutil/recurse.c b/libutil/recurse.c
index f50347e..41dd54b 100644
--- a/libutil/recurse.c
+++ b/libutil/recurse.c
_AT_@ -16,27 +16,30 @@
 int recurse_status = 0;
 
 void
-recurse(const char *path, void *data, struct recursor *r)
+recurse(int dirfd, const char *name, void *data, struct recursor *r)
 {
 	struct dirent *d;
 	struct history *new, *h;
 	struct stat st, dst;
 	DIR *dp;
-	char subpath[PATH_MAX];
-	int flags = 0;
+	int flags = 0, fd;
+	size_t pathlen = r->pathlen;
+
+	if (dirfd == AT_FDCWD)
+		pathlen = estrlcpy(r->path, name, sizeof(r->path));
 
 	if (r->follow == 'P' || (r->follow == 'H' && r->depth))
 		flags |= AT_SYMLINK_NOFOLLOW;
 
-	if (fstatat(AT_FDCWD, path, &st, flags) < 0) {
+	if (fstatat(dirfd, name, &st, flags) < 0) {
 		if (!(r->flags & SILENT)) {
-			weprintf("stat %s:", path);
+			weprintf("stat %s:", r->path);
 			recurse_status = 1;
 		}
 		return;
 	}
 	if (!S_ISDIR(st.st_mode)) {
-		r->fn(path, &st, data, r);
+		r->fn(dirfd, name, &st, data, r);
 		return;
 	}
 
_AT_@ -51,35 +54,39 @@ recurse(const char *path, void *data, struct recursor *r)
 			return;
 
 	if (!r->depth && (r->flags & DIRFIRST))
-		r->fn(path, &st, data, r);
+		r->fn(dirfd, name, &st, data, r);
 
 	if (!r->maxdepth || r->depth + 1 < r->maxdepth) {
-		if (!(dp = opendir(path))) {
+		fd = openat(dirfd, name, O_RDONLY | O_CLOEXEC | O_DIRECTORY);
+		if (fd < 0) {
+			weprintf("open %s:", r->path);
+			recurse_status = 1;
+		}
+		if (!(dp = fdopendir(fd))) {
 			if (!(r->flags & SILENT)) {
-				weprintf("opendir %s:", path);
+				weprintf("fdopendir:");
 				recurse_status = 1;
 			}
 			return;
 		}
+		if (r->path[pathlen - 1] != '/')
+			pathlen += estrlcpy(r->path + pathlen, "/", sizeof(r->path) - pathlen);
+		if (r->follow == 'H')
+			flags |= AT_SYMLINK_NOFOLLOW;
 		while ((d = readdir(dp))) {
-			if (r->follow == 'H')
-				flags |= AT_SYMLINK_NOFOLLOW;
 			if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 				continue;
-			estrlcpy(subpath, path, sizeof(subpath));
-			if (path[strlen(path) - 1] != '/')
-				estrlcat(subpath, "/", sizeof(subpath));
-			estrlcat(subpath, d->d_name, sizeof(subpath));
-			if (fstatat(AT_FDCWD, subpath, &dst, flags) < 0) {
+			r->pathlen = pathlen + estrlcpy(r->path + pathlen, d->d_name, sizeof(r->path) - pathlen);
+			if (fstatat(fd, d->d_name, &dst, flags) < 0) {
 				if (!(r->flags & SILENT)) {
-					weprintf("stat %s:", subpath);
+					weprintf("stat %s:", r->path);
 					recurse_status = 1;
 				}
 			} else if ((r->flags & SAMEDEV) && dst.st_dev != st.st_dev) {
 				continue;
 			} else {
 				r->depth++;
-				r->fn(subpath, &dst, data, r);
+				r->fn(fd, d->d_name, &dst, data, r);
 				r->depth--;
 			}
 		}
_AT_@ -88,7 +95,7 @@ recurse(const char *path, void *data, struct recursor *r)
 
 	if (!r->depth) {
 		if (!(r->flags & DIRFIRST))
-			r->fn(path, &st, data, r);
+			r->fn(dirfd, name, &st, data, r);
 
 		for (; r->hist; ) {
 			h = r->hist;
diff --git a/libutil/rm.c b/libutil/rm.c
index 11c20fb..8d4be08 100644
--- a/libutil/rm.c
+++ b/libutil/rm.c
_AT_@ -2,6 +2,7 @@
 #include <sys/stat.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <stdio.h>
 #include <unistd.h>
 
_AT_@ -11,20 +12,20 @@
 int rm_status = 0;
 
 void
-rm(const char *path, struct stat *st, void *data, struct recursor *r)
+rm(int dirfd, const char *name, struct stat *st, void *data, struct recursor *r)
 {
 	if (!r->maxdepth && S_ISDIR(st->st_mode)) {
-		recurse(path, NULL, r);
+		recurse(dirfd, name, NULL, r);
 
-		if (rmdir(path) < 0) {
+		if (unlinkat(dirfd, name, AT_REMOVEDIR) < 0) {
 			if (!(r->flags & SILENT))
-				weprintf("rmdir %s:", path);
+				weprintf("rmdir %s:", r->path);
 			if (!((r->flags & SILENT) && errno == ENOENT))
 				rm_status = 1;
 		}
-	} else if (unlink(path) < 0) {
+	} else if (unlinkat(dirfd, name, 0) < 0) {
 		if (!(r->flags & SILENT))
-			weprintf("unlink %s:", path);
+			weprintf("unlink %s:", r->path);
 		if (!((r->flags & SILENT) && errno == ENOENT))
 			rm_status = 1;
 	}
diff --git a/mv.c b/mv.c
index 2cf98d1..c93f72d 100644
--- a/mv.c
+++ b/mv.c
_AT_@ -2,6 +2,7 @@
 #include <sys/stat.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <stdio.h>
 
 #include "fs.h"
_AT_@ -21,7 +22,7 @@ mv(const char *s1, const char *s2, int depth)
 		cp_aflag = cp_rflag = cp_pflag = 1;
 		cp_follow = 'P';
 		cp(s1, s2, depth);
-		recurse(s1, NULL, &r);
+		recurse(AT_FDCWD, s1, NULL, &r);
 		return (mv_status = cp_status || rm_status);
 	}
 	mv_status = 1;
diff --git a/rm.c b/rm.c
index 42bc022..b776d0f 100644
--- a/rm.c
+++ b/rm.c
_AT_@ -1,4 +1,6 @@
 /* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
+
 #include "fs.h"
 #include "util.h"
 
_AT_@ -34,7 +36,7 @@ main(int argc, char *argv[])
 	}
 
 	for (; *argv; argc--, argv++)
-		recurse(*argv, NULL, &r);
+		recurse(AT_FDCWD, *argv, NULL, &r);
 
 	return rm_status || recurse_status;
 }
diff --git a/tar.c b/tar.c
index f9158c2..603e544 100644
--- a/tar.c
+++ b/tar.c
_AT_@ -368,14 +368,14 @@ print(char *fname, ssize_t l, char b[BLKSIZ])
 }
 
 static void
-c(const char *path, struct stat *st, void *data, struct recursor *r)
+c(int dirfd, const char *name, struct stat *st, void *data, struct recursor *r)
 {
-	archive(path);
+	archive(r->path);
 	if (vflag)
-		puts(path);
+		puts(r->path);
 
 	if (S_ISDIR(st->st_mode))
-		recurse(path, NULL, r);
+		recurse(dirfd, name, NULL, r);
 }
 
 static void
_AT_@ -578,7 +578,7 @@ main(int argc, char *argv[])
 		if (chdir(dir) < 0)
 			eprintf("chdir %s:", dir);
 		for (; *argv; argc--, argv++)
-			recurse(*argv, NULL, &r);
+			recurse(AT_FDCWD, *argv, NULL, &r);
 		break;
 	case 't':
 	case 'x':
-- 
2.24.1
Received on Mon Dec 23 2019 - 02:03:59 CET

This archive was generated by hypermail 2.3.0 : Mon Dec 23 2019 - 02:48:23 CET