From: Derrick Stolee <stolee@gmail.com>
Date: Wed, 25 Sep 2019 13:05:12 GMT
Subject: Re: [PATCH v2 11/19] hashmap_get_next returns "struct hashmap_entry *"
Message-ID: <02e90301-9d50-4105-5988-853d1018ba30@gmail.com>
In-Reply-To: <20190924010324.22619-12-e@80x24.org>

On 9/23/2019 9:03 PM, Eric Wong wrote:
> This is a step towards removing the requirement for
> hashmap_entry being the first field of a struct.
> 
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
>  diff.c                  | 19 ++++++++++++-------
>  diffcore-rename.c       | 11 +++++++----
>  hashmap.c               |  2 +-
>  hashmap.h               | 12 ++++++++----
>  name-hash.c             |  8 +++++---
>  t/helper/test-hashmap.c | 10 ++++++----
>  6 files changed, 39 insertions(+), 23 deletions(-)
> 
> diff --git a/diff.c b/diff.c
> index 72d3c6aa19..663b5d01f8 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1035,8 +1035,10 @@ static void pmb_advance_or_null_multi_match(struct diff_options *o,
>  {
>  	int i;
>  	char *got_match = xcalloc(1, pmb_nr);
> +	struct hashmap_entry *ent = &match->ent;
>  
> -	for (; match; match = hashmap_get_next(hm, &match->ent)) {
> +	for (; ent; ent = hashmap_get_next(hm, ent)) {

I suppose that the old code had a blank first entry in the for(;;),
but we could move our `ent = &match->ent` into the initializer, right?
That would make the loop look a little better, maybe. This happens
again below.

Thanks,
-Stolee

> @@ -1189,8 +1193,9 @@ static void mark_color_as_moved(struct diff_options *o,
>  			 * The current line is the start of a new block.
>  			 * Setup the set of potential blocks.
>  			 */
> -			for (; match; match = hashmap_get_next(hm,
> -								&match->ent)) {
> +			for (; ent; ent = hashmap_get_next(hm, ent)) {
> +				match = container_of(ent, struct moved_entry,
> +							ent);