Hi, Will

> On Tue, Sep 16, 2025 at 09:19:30PM +0100, Yeoreum Yun wrote:
> > > On Wed, Sep 03, 2025 at 04:00:19PM +0100, Yeoreum Yun wrote:
> > > > +       switch (kasan_arg_write_only) {
> > > > +       case KASAN_ARG_WRITE_ONLY_DEFAULT:
> > > > +               /* Default is specified by kasan_flag_write_only 
> > > > definition. */
> > > > +               break;
> > > > +       case KASAN_ARG_WRITE_ONLY_OFF:
> > > > +               kasan_flag_write_only = false;
> > > > +               break;
> > > > +       case KASAN_ARG_WRITE_ONLY_ON:
> > > > +               kasan_flag_write_only = true;
> > > > +               break;
> > > > +       }
> > > > +
> > > >         kasan_init_tags();
> > >
> > > I'm probably missing something here, but why have 'enum
> > > kasan_arg_write_only' at all? What stops you from setting
> > > 'kasan_flag_write_only' directly from early_kasan_flag_write_only()?
> > >
> > > This all looks weirdly over-engineered, as though 'kasan_flag_write_only'
> > > is expected to be statically initialised to something other than 'false'.
> >
> > For the conherent pattern for other options.
> > Since other options manage arg value and internal state separately,
> > I just followed former ancestor.
>
> I'm not sure it's the best option to blindly follow the existing code
> here. To pick another kasan "mode" at random, 'kasan_flag_vmalloc' is
> initialised differently depending on CONFIG_KASAN_VMALLOC and so
> allowing for the default value to differ based on the kernel
> configuration makes sense.
>
> But that doesn't apply here.
>
> I'd recommend starting simple and just having the 'flag', especially as
> you already made a small mistake because of mixing up the 'flag' with
> the 'arg'.

Okay. I'll change this.
Thanks.

--
Sincerely,
Yeoreum Yun

Reply via email to