Messages in this thread | | | From | Alexander Potapenko <> | Date | Tue, 12 Jan 2021 08:53:22 +0100 | Subject | Re: [PATCH 03/11] kasan: clean up comments in tests |
| |
On Tue, Jan 5, 2021 at 7:28 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > Clarify and update comments and info messages in KASAN tests. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > 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? > > @@ -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.
> 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?
> 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.
Alex
| |