Messages in this thread | | | From | Daniel Latypov <> | Date | Fri, 13 Jan 2023 14:45:29 -0800 | Subject | Re: [PATCH v1] lib/hashtable_test.c: add test for the hashtable structure |
| |
On Fri, Jan 13, 2023 at 2:23 PM Rae Moar <rmoar@google.com> wrote:
<snip>
> > Note: given x is supposed to point to a or b, I don't know if checking > > against a.data does us much good. > > If we're trying to check that hash_add() doesn't mutate the keys and > > data, this code won't catch it. > > We'd have to instead do something like > > if(x->key != 1 && x->key != 2) KUNIT_FAIL(test, ...); > > > > This seems like a good change to me in combination with changing it to > x->visited++;. > Although David's suggestion might be slightly more exhaustive. > Why wouldn't it be important to check that the key matches the data?
Checks like KUNIT_EXPECT_EQ(test, x->data, a.data); won't do anything, given that x == &a. We're just comparing x->data to itself.
So we would have to write something instead like hash_for_each(hash, bkt, x, node) { x->visited++; if (x->key == a.key) { KUNIT_EXPECT_EQ(test, x->data, 13); } else if (x->key == b.key) { KUNIT_EXPECT_EQ(test, x->data, 10); } else { /* some call to KUNIT_FAIL about a bad key */ } }
Maybe that's worth it in one of the test cases, but I don't know if it's necessary to replicate this in the other places where we're incrementing `visited` by checking keys.
Daniel
| |