Re: [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes
To
Johannes Schindelin
Cc
Junio C Hamano
Derrick Stolee
Phillip Wood
git@vger.kernel.org
From
Eric Wong
See Also
Prev Ref 1
Date
2019-09-30 10:01:38 UTC
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi Eric,
> 
> On Tue, 24 Sep 2019, Eric Wong wrote:
> 
> > Patches 1-11 are largely unchanged from the original series with the
> > exception of 2, which is new and posted at:
> >
> > 	https://public-inbox.org/git/20190908074953.kux7zz4y7iolqko4@whir/
> >
> > 12-17 take further steps to get us away from hashmap_entry being
> > the first element, but they're also a bit ugly because __typeof__
> > isn't portable
> >
> > 18-19 finally brings me to the APIs I want to expose without
> > relying on __typeof :)
> 
> I won't have time to review this patch series, but I wanted to throw out
> the idea of storing the offset in `struct hashmap`, i.e. the offset of
> the `struct hashmap_entry` in the actual entry struct?
> 
> Example:
> 
> 	struct erics_entry {
> 		const char *i_want_this_to_be_the_first_field;
> 		struct hashmap_entry e;
> 		[...]
> 	};
> 
> 	[...]
> 
> 	struct erics_entry *dummy = NULL;
> 	size_t offset = ((char *)dummy->e) - ((char *)dummy);
> 	[... store that offset in the hashmap and subtract it from the
> 	stored pointers before returning an entry...]
> 
> IOW rather than assuming that it is the first field, we could allow an
> offset other than 0. I'm just not sure how to implement this elegantly,
> but it should be much easier to do this portably than the `typeof()`
> approach.

Yeah, that needs to be &dummy->e, but that's the OFFSETOF_VAR
macro in patch 18.  I think it's quite elegant, but I need to
fix a clang warning for it...