Re: [PATCH 5/8] symlinks: do not include current working directory in dir removal

["René Scharfe" <l.s.r@web.de>]

Am 21.11.21 um 01:46 schrieb Elijah Newren via GitGitGadget:
> From: Elijah Newren <newren@gmail.com>
>
> symlinks has a pair of schedule_dir_for_removal() and
> remove_scheduled_dirs() functions that ensure that directories made
> empty by removing other files also themselves get removed.  However, we
> want to exclude the current working directory and leave it around so
> that subsequent git commands (and non-git commands) that the user runs
> afterwards don't cause the user to get confused.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  symlinks.c           | 12 +++++++++++-
>  t/t2501-cwd-empty.sh | 12 ++++++------
>  2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/symlinks.c b/symlinks.c
> index 5232d02020c..84622bedcde 100644
> --- a/symlinks.c
> +++ b/symlinks.c
> @@ -275,11 +275,18 @@ static int threaded_has_dirs_only_path(struct cache_def *cache, const char *name
>
>  static struct strbuf removal = STRBUF_INIT;
>
> +static int cant_remove(char *dirname)
> +{
> +	if (the_cwd && !strcmp(dirname, the_cwd))

Initializing the_cwd to an empty string would allow removing the NULL check
everywhere.

Is strcmp() sufficient or do we need fspathcmp() in these kinds of checks?
Do we need to worry about normalizing directory separators?

> +		return 1;
> +	return rmdir(dirname);
> +}

I wouldn't expect a function of that name to actually try to remove
the directory.  Or with that body to require a non-const dirname.
It's used only once, perhaps inline it?

> +
>  static void do_remove_scheduled_dirs(int new_len)
>  {
>  	while (removal.len > new_len) {
>  		removal.buf[removal.len] = '\0';
> -		if (rmdir(removal.buf))
> +		if (cant_remove(removal.buf))
>  			break;
>  		do {
>  			removal.len--;
> @@ -293,6 +300,9 @@ void schedule_dir_for_removal(const char *name, int len)
>  {
>  	int match_len, last_slash, i, previous_slash;
>
> +	if (the_cwd && !strcmp(name, the_cwd))
> +		return;	/* Do not remove the current working directory */
> +
>  	match_len = last_slash = i =
>  		longest_path_match(name, len, removal.buf, removal.len,
>  				   &previous_slash);