On Sat, Oct 19, 2019 at 1:27 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Fri, Oct 18, 2019 at 02:55:49PM -0700, David Gow wrote: > > + list4 = kzalloc(sizeof(*list4), GFP_KERNEL); > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, list4); > > Why not just use GFP_KERNEL | GFP_NOFAIL and remove the check? I've sent a new version of the patch out (v5) which uses __GFP_NOFAIL instead. The idea had been to exercise KUnit's assertion functionality, in the hope that it'd allow the test to fail (but potentially allow other tests to still run) in the case of allocation failure. Given that we're only allocating enough to store ~4 pointers in total, though, that's probably of little use. > kzalloc() can't return error pointers. If this were an IS_ERR_OR_NULL() > check then it would generate a static checker warning, but static > checkers don't know about KUNIT_ASSERT_NOT_ERR_OR_NULL() yet so you're > safe. Alas, KUnit doesn't have a KUNIT_ASSERT_NOT_NULL() macro, and I'd assumed it was not dangerous (even if not ideal) to check for error pointers, even if kzalloc() can't return them. Perhaps it'd make sense to add a convenient way of checking the NULL-ness of pointers to KUnit (it's possible with the KUNIT_ASSERT_PTR_EQ(), but requires a bit of casting to make the type checker happy) in the future. Once KUnit is properly upstream, it may be worth teaching the static analysis tools about these functions to avoid having warnings in these sorts of tests. For now, though, (and for this test in particular), I agree with the suggestion of just using __GFP_NOFAIL. Thanks a lot for the comments, -- David
