Re: [PATCH v2 08/17] libtraceeval histograms: Move hash functions into their own file

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 15 Aug 2023 16:23:33 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> > > +__hidden void hash_add(struct hash_table *hash, struct hash_item *item, unsigned key)
> > > +{
> > > +	key &= HASH_MASK(hash->bits);    
> > 
> > key should already be masked to HASH_MASK(hash->bits) via make_hash().  If
> > those bits are set, we have a bug somewhere.
> > 
> > I think it's better to check to see if those bits are set and bail out loudly
> > with an error.  
> 
> I debated a bit about where to do the mask. I didn't want to put a
> dependency that the bits were already masked, so I just did it twice. It's
> a very fast operation.
> 
> I also don't want to make the dependency that key can only come from
> mask_hash(). As it's critical that key is masked here (or we have a array
> overflow), I just kept it in both places.
> 
> I can comment that here.

Thinking about this more, I'm not even sure I want HASH_MASK() in
make_hash(). In fact, I think I'm going to remove it. The hash size is
really internal to the hash code, and to maintain a proper abstraction,
other code should not assume how big the bucket array is.

-- Steve