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