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? Also, there is a small typo in the parameter name here: @lockdebug_ok is used instead of @lockdep_ok. > +void add_taint_module(struct module *mod, unsigned flag, > + enum lockdep_ok lockdep_ok) > { > add_taint(flag, lockdep_ok); > - set_bit(flag, &mod->taints); > + if (mod) > + set_bit(flag, &mod->taints); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
