Re: [PATCH v2 07/24] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 9, 2017, 23:36 UTC
Message-ID: <20171009233615.GK19555@aiede.mtv.corp.google.com>
In-Reply-To: <20171009011132.675341-8-sandals@crustytoothpaste.net>

brian m. carlson wrote:

Show 19 quoted lines

> All of the callers already pass the hash member of struct object_id, so
> update them to pass a pointer to the struct directly,
>
> This transformation was done with an update to declaration and
> definition and the following semantic patch:
>
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_refdup(E1, E2, E3.hash, E4)
> + resolve_refdup(E1, E2, &E3, E4)
>
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_refdup(E1, E2, E3->hash, E4)
> + resolve_refdup(E1, E2, E3, E4)
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>

Lovely. I tried putting that in contrib/coccinelle/resolve_refdup.cocci, running

  git grep -l -e resolve_refdup -- '*.c' '*.h' |
  xargs spatch --in-place --sp-file contrib/coccinelle/resolve_refdup.cocci

and diffing the result against this commit. With --word-diff, there are a small number of changes:

- the above semantic patch handles resolve_refdup but not
  refs_resolve_refdup.  This commit does both.

- as mentioned in the commit message, the above semantic patch only
  updates callers.  This commit updates the implementations to match.

Without --word-diff, I also see some line-wrapping changes, which all seem reasonable. (Coccinelle's line-wrapping heuristics seem to be pretty specific to Linux kernel style.)

In other words, this does what it says on the cover in a straightforward and reviewable way. Thanks for that.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>