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

It reads correct to me.

ok tb

Only bikesheds below.

> +	p = strchr(path, '\0');

If you add a check

	if (p == path)
		return 1;

naddy's concern would be addressed. I'd prefer that over referring
to 'rmdir("") doesn't succeed anyway' or other sorts of cleverness.
(If someone copies this chunk of code, they might not notice this
assumption and a check in code is always better than a comment.)

> +	while (--p > path && *p == '/')
> +		continue;
> +
> +	while (p > path) {

this while feels more like a for loop to me:

	for (; p > path; p--) {

> +		if (*p == '/' && p[-1] != '/') {

This may be just me but I'd much rather people either use dereferencing
or indexing and refrain from mixing them. That is, please either use
p[0] and p[-1] or *p and *(p - 1) if you don't mind.

> +			*p = '\0';

and adjust this to "p[0] = '\0';" if you choose indexing.

> +			if (rmdir(path) == -1) {
> +				warn("%s", path);
> +				return 1;
> +			}
>  		}
> +		p--;
>  	}
> -
> -	return (0);
> +	return 0;
>  }
>  
>  static void __dead