From: Christian Weisgerber <naddy@mips.inka.de>
Subject: Re: Fix rmdir -p /foo/bar
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Sun, 11 May 2025 20:35:37 +0200

Theo Buehler:

> > -		while (--p > path && *p == '/')
> > +		while (--p >= path && *p == '/')
> >  			continue;
> 
> I don't think this approach is kosher. p can now end up pointing before
> path, which is UB.

Is it UB?  Those are all pointers, not arrays.

> I think NetBSD's solution is cleaner, if slightly
> less efficient:
> 
> https://github.com/NetBSD/src/blob/trunk/bin/rmdir/rmdir.c#L100

Diff for the sake of completeness below, but...

If you do "rmdir foo///bar", it will call rmdir("foo//") and only
then delete trailing slashes.  But that means that it can try to
delete the root directory:

# rmdir -p ///.ssh/agent               
rmdir: //: Is a directory


diff 3aaa63eb46949490a39db9c6d82aacc8ee5d8551 a222ff3fb1ae705e9a81c1f0c991c1cf1978bc9f
commit - 3aaa63eb46949490a39db9c6d82aacc8ee5d8551
commit + a222ff3fb1ae705e9a81c1f0c991c1cf1978bc9f
blob - 0197254749acbef1496562627053a7b80fdfd5ec
blob + e87fb4f4de22caee3084e7a66460b4d3a99e13fe
--- bin/rmdir/rmdir.c
+++ bin/rmdir/rmdir.c
@@ -67,14 +67,7 @@ main(int argc, char *argv[])
 		usage();
 
 	for (errors = 0; *argv; argv++) {
-		char *p;
-
-		/* Delete trailing slashes, per POSIX. */
-		p = *argv + strlen(*argv);
-		while (--p > *argv && *p == '/')
-			continue;
-		*++p = '\0';
-
+		/* We rely on the kernel to ignore trailing '/' characters. */
 		if (rmdir(*argv) == -1) {
 			warn("%s", *argv);
 			errors = 1;
@@ -91,11 +84,15 @@ rm_path(char *path)
 	char *p;
 
 	while ((p = strrchr(path, '/')) != NULL) {
-		/* Delete trailing slashes. */
-		while (--p > path && *p == '/')
+		*p = 0;
+		if (p[1] == 0)
+			/* Ignore trailing '/' on deleted name */
 			continue;
-		*++p = '\0';
 
+		if (*path == 0)
+			/* At top level (root) directory */
+			break;
+
 		if (rmdir(path) == -1) {
 			warn("%s", path);
 			return (1);
-- 
Christian "naddy" Weisgerber                          naddy@mips.inka.de