On Tue, Jan 12, 2021 at 8:53 AM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > On Tue, Jan 5, 2021 at 7:28 PM Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote: > > > > Clarify and update comments and info messages in KASAN tests. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx> > > Link: https://linux-review.googlesource.com/id/I6c816c51fa1e0eb7aa3dead6bda1f339d2af46c8 > > > void *kasan_ptr_result; > > int kasan_int_result; > Shouldn't these two variables be static, by the way? No, then the compiler starts eliminating accesses. > > @@ -39,14 +38,13 @@ static struct kunit_resource resource; > > static struct kunit_kasan_expectation fail_data; > > static bool multishot; > > > > +/* > > + * Temporarily enable multi-shot mode. Otherwise, KASAN would only report the > > + * first detected bug and panic the kernel if panic_on_warn is enabled. > > + */ > > YMMV, but I think this comment was at its place already. It gets updated by one of the subsequent patches. > > static int kasan_test_init(struct kunit *test) > > { > > - /* > > - * Temporarily enable multi-shot mode and set panic_on_warn=0. > > - * Otherwise, we'd only get a report for the first case. > > - */ > > multishot = kasan_save_enable_multi_shot(); > > Unrelated to this change, but have you considered storing > test-specific data in test->priv instead of globals? I'd say that test->priv is for some per-test data that's used in the tests, and multishot is not a part of that. > > if (!IS_ENABLED(CONFIG_SLUB)) { > > - kunit_info(test, "CONFIG_SLUB is not enabled."); > > + kunit_info(test, "skipping, CONFIG_SLUB required"); > > return; > > } > > You may want to introduce a macro that takes a config name and prints > the warning/returns if it's not enabled. Good idea, will do in v2. Thanks!
