Re: [PATCH 11/11] hashmap_get_next returns "struct hashmap_entry *"
- From
- Eric Wong <e@80x24.org>
- Date
- Aug 30, 2019, 19:36 UTC
- Message-ID
- <20190830193636.xg6pcd5hjcgk3nob@dcvr>
- In-Reply-To
- <2f22841c-779e-6d6e-19b7-0a119088e343@gmail.com>
Derrick Stolee <stolee@gmail.com> wrote:
Show 16 quoted lines
> On 8/25/2019 10:43 PM, Eric Wong wrote:
> > --- 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)) {
> > + match = container_of(ent, struct moved_entry, ent);
>
> Lines like this are very difficult to parse. In this
> container_of() macro, 'ent' is taking both the 'ptr' and
> 'member' values.Agreed, naming is hard :<
In the Linux kernel list.h implementation, there's actually list_for_each_entry, list_next_entry and a bunch of other macros which allow the caller to avoid using container_of. We only have list_first_entry, so far.
We can draw inspiration from those macros by creating hashmap_get_next_entry and hashmap_for_each_entry macros which allow callers specify the type once; and there'd be no need for callers to specify the hashmap_entry pointer name at all :)
Unlike the kernel, it looks like we can't rely on __typeof__ in git, but I think we can let the caller specify the type once...