On Fri, Sep 04, 2026 at 05:21:22AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
> to consider:
> - [Medium] The newly added docstring for `add_taint_module` provides 
> dangerously misleading advice regarding the `lockdep_ok` parameter, treating 
> an enum as a boolean and effectively reversing its logical meaning.
> --
> 
> commit 2864484c56cc4bda3a7f73ba92dcb0a54f6a1232
> Author: Greg Kroah-Hartman <[email protected]>
> Subject: module: pull out add_taint_module() to be public
> 
> This commit makes add_taint_module() public so other kernel code can call
> it, providing a fallback to add_taint() when modules are not enabled.
> 
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index d0e1e0bd2ad06..444d990c99832 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> [ ... ]
> > +/**
> > + * add_taint_module: add a taint flag if not already set for a specific 
> > module
> > + * @mod: pointer to the module that caused the problem
> > + * @flag: one of the TAINT_* constants.
> > + * @lockdep_ok: whether lock debugging is still OK.
> > + *
> > + * If something bad has gone wrong, you'll want @lockdebug_ok = false, but 
> > for
> > + * some noteworthy-but-not-corrupting cases, it can be set to true.
> > + */
> 
> [Severity: Medium]
> Does this documentation invert the logical meaning of the lockdep_ok enum?
> 
> Since lockdep_ok is an enum where LOCKDEP_STILL_OK is 0 and
> LOCKDEP_NOW_UNRELIABLE is 1, a developer following this advice and passing
> false (0) when a serious error occurs will inadvertently leave lock
> debugging enabled. Passing true (1) for minor issues will unintentionally
> disable lock debugging.
> 
> Would it be clearer to document the explicit enum values rather than
> suggesting boolean true and false?

This preserves the existing api, nothing is changed, if we wish to
change this later we can.

> Also, there is a small typo in the parameter name here: @lockdebug_ok
> is used instead of @lockdep_ok.

That comes straight from the add_taint() documentation, so we are
typo-compatible :)


Reply via email to