Re: [PATCH v1 3/7] mv: free the *with_slash in check_dir_in_index()

From: Derrick Stolee <derrickstolee@github.com>
Date: Jul 19, 2022, 17:46 UTC
Message-ID: <0a93bea5-2d5a-298d-f501-ec4bb9a9c45f@github.com>
In-Reply-To: <20220719132809.409247-4-shaoxuan.yuan02@gmail.com>

On 7/19/2022 9:28 AM, Shaoxuan Yuan wrote:> *with_slash may be a malloc'd pointer, and when it is, free it. It might be worth mentioning that you are reorganizing the method to use gotos.

>  static int check_dir_in_index(const char *name)
>  {
> +	int ret = 1;

Interesting to "assume error", but that works since there is only one place where the success is guaranteed.

Should we use -1 here?

>  		if (ce_skip_worktree(ce))
> -			return 0;
> +			ret = 0;

This could be

	ret = ce_skip_worktree(ce);

unless you really rely on the "1" value.

Show 6 quoted lines

>  	}
> -	return 1;
> +
> +free_return:
> +	if (with_slash != name)
> +		free((char *)with_slash);

Good check to not clear 'name'. It's unfortunate that we need to cast here, but it makes sense.

Thanks, -Stolee