Index | Thread | Search

From:
Christian Weisgerber <naddy@mips.inka.de>
Subject:
Re: Fix rmdir -p /foo/bar
To:
Ingo Schwarze <schwarze@usta.de>
Cc:
Theo Buehler <tb@theobuehler.org>, tech@openbsd.org
Date:
Mon, 12 May 2025 21:17:20 +0200

Download raw body.

Thread
Ingo Schwarze:

>  * The /* Delete trailing slashes, per POSIX. */ part is actually
>    a POSIX violation and must be removed completely.

Yes, both FreeBSD and NetBSD also discovered this.

>  * The easiest way to fix rm_path() actually is to rewrite it
>    completely. [...]

After staring at it far too long, I have now convinced myself that
this is indeed correct for all corner cases.  OK naddy@, but somebody
else should also look over it carefully.

A small remark:

>  int
>  rm_path(char *path)
>  {
>  	char *p;
>  
> -	while ((p = strrchr(path, '/')) != NULL) {
> -		/* Delete trailing slashes. */
> -		while (--p > path && *p == '/')
> -			continue;
> -		*++p = '\0';
> -
> -		if (rmdir(path) == -1) {
> -			warn("%s", path);
> -			return (1);
> +	p = strchr(path, '\0');
> +	while (--p > path && *p == '/')
> +		continue;

So if we were ever to pass a zero-length string, this would result
in UB, but since rm_path() will never be called with "", it is fine,
I guess?

> +
> +	while (p > path) {
> +		if (*p == '/' && p[-1] != '/') {
> +			*p = '\0';
> +			if (rmdir(path) == -1) {
> +				warn("%s", path);
> +				return 1;
> +			}
>  		}
> +		p--;
>  	}
> -
> -	return (0);
> +	return 0;
>  }

-- 
Christian "naddy" Weisgerber                          naddy@mips.inka.de