On Tue, 15 Aug 2023 16:23:33 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> 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
|