Subject: Re: [PATCH 3/5] use SWAP macro

From: Junio C Hamano <[email-masked]>
Date: Mon, 30 Jan 2017 14:22:10 -0800

René Scharfe <[email-masked]> writes:

>  		if (tree2->flags & UNINTERESTING) {
> -			struct object *tmp = tree2;
> -			tree2 = tree1;
> -			tree1 = tmp;
> +			SWAP(tree2, tree1);

A human would have written this SWAP(tree1, tree2).

Not that I think such a manual fix-up should be made in _this_
patch, which may end up mixing mechanical conversion (which we may
want to keep reproducible) and hand tweaks.  But this swapped swap
reads somewhat silly.

> diff --git a/diff-no-index.c b/diff-no-index.c
> index f420786039..1ae09894d7 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -186,9 +186,8 @@ static int queue_diff(struct diff_options *o,
>  
>  		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
>  			unsigned tmp;
> -			const char *tmp_c;
>  			tmp = mode1; mode1 = mode2; mode2 = tmp;
> -			tmp_c = name1; name1 = name2; name2 = tmp_c;
> +			SWAP(name1, name2);

Curious that mode swapping is left for a later iteration.

> diff --git a/diff.c b/diff.c
> index f08cd8e033..9de1ba264f 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5118,13 +5118,11 @@ void diff_change(struct diff_options *options,
>  
>  	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
>  		unsigned tmp;
> -		const unsigned char *tmp_c;
> -		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
> -		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
> +		SWAP(old_mode, new_mode);
> +		SWAP(old_sha1, new_sha1);
>  		tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
>  			new_sha1_valid = tmp;

So is this one.

> diff --git a/merge-recursive.c b/merge-recursive.c
> ...
> -			tmp = ren2;
> -			ren2 = ren1;
> -			ren1 = tmp;
> +			SWAP(ren2, ren1);

A human would have written this SWAP(ren1, ren2).