From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 18 Jul 2011 23:39:00 GMT
Subject: Re: [PATCH 19/48] merge-recursive: Remember to free generated unique path names
Message-ID: <7voc0rcg6j.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1307518278-23814-20-git-send-email-newren@gmail.com>

Elijah Newren <newren@gmail.com> writes:

> @@ -1433,12 +1434,17 @@ static int process_df_entry(struct merge_options *o,
>  		}
>  	} else if (o_sha && (!a_sha || !b_sha)) {
>  		/* Modify/delete; deleted side may have put a directory in the way */
> -		const char *new_path = path;
> -		if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode))
> +		char *new_path;
> +		int free_me = 0;
> +		if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
>  			new_path = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
> +			free_me = 1;
> +		}
>  		clean_merge = 0;
> -		handle_delete_modify(o, path, new_path,
> +		handle_delete_modify(o, path, free_me ? new_path : path,
>  				     a_sha, a_mode, b_sha, b_mode);

Here free_me is not about free_me but is used as a substitute for "did I
stuff the path to be used in new_path, or should I use the original path?",
and I find the variable misnamed.  How about doing it this way instead?

	char *renamed = NULL;
        if (lstat(path, &st) ...)
        	renamed = unique_path(...);
	clean_merge = 0;
	handle_delete_modify(o, path, renamed ? renamed : path, ...);

> +		if (free_me)
> +			free(new_path);

and then you can free renamed unconditoinally here.

Other than that looks good to me.

Thanks.