On 7/20/26 2:25 PM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Low] Mixing goto-based cleanup labels with scope-based cleanup > (scoped_guard) in the same function. > -- > > --- Patch [3]: [PATCH 3/5] module/dups: Use scoped guards for kmod_dup_mutex > --- > commit 3299ca2ac8ae46392f412ae7c186f4907e7f8201 > Author: Petr Pavlu <[email protected]> > > module/dups: Use scoped guards for kmod_dup_mutex > > This patch replaces explicit mutex_lock/unlock calls for kmod_dup_mutex > with scoped_guard to shorten the code and clarify the lock's scope. > >> diff --git a/kernel/module/dups.c b/kernel/module/dups.c >> index f0bdd7560d7e7..a62619ec057ed 100644 >> --- a/kernel/module/dups.c >> +++ b/kernel/module/dups.c > > [ ... ] > >> @@ -122,10 +121,13 @@ bool kmod_dup_request_exists_wait(char *module_name, >> bool wait, int *dup_ret) >> INIT_DELAYED_WORK(&new_kmod_req->delete_work, kmod_dup_request_delete); >> init_completion(&new_kmod_req->first_req_done); >> >> - mutex_lock(&kmod_dup_mutex); >> + scoped_guard(mutex, &kmod_dup_mutex) { >> + kmod_req = kmod_dup_request_lookup(module_name); >> + if (kmod_req) { >> + get_kmod_req(kmod_req); >> + goto dup_req; >> + } >> >> - kmod_req = kmod_dup_request_lookup(module_name); >> - if (!kmod_req) { > > [Severity: Low] > Does this mix scope-based cleanup with goto-based cleanup in the same > function? > > Looking further down in kmod_dup_request_exists_wait(), there are still > several goto statements that jump to an out label for manual cleanup: > > if (!wait) { > /* ... */ > *dup_ret = 0; > goto out; > } > > ret = wait_for_completion_state(&kmod_req->first_req_done, > TASK_KILLABLE); > if (ret) { > *dup_ret = ret; > goto out; > } > > *dup_ret = kmod_req->dup_ret; > > out: > put_kmod_req(kmod_req); > return true; > > According to include/linux/cleanup.h, the expectation is that usage of > "goto" and cleanup helpers is never mixed in the same function. All resources > that need a "goto" cleanup should either be converted to scope-based cleanup, > or none of them should. > > Should put_kmod_req() be converted to use scope-based cleanup as well, or > should this function avoid using scoped_guard() to keep the cleanup pattern > consistent? >
Fair, I'll clean this up. Sashiko also reported the same issue in a reply to patch #5. -- Petr
