From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dynamic_debug: use KBUILD_MODFILE for unique builtin module names Date: Thu, 04 Jun 2026 12:12:52 +1000 Message-ID: In-Reply-To: <20260602-dd-maint-2-v4-25-19a1445585a8@gmail.com> References: <20260602-dd-maint-2-v4-0-19a1445585a8@gmail.com> <20260602-dd-maint-2-v4-25-19a1445585a8@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review The key behavioral change in the series. For builtins, `KBUILD_MODFILE` (e.g., `init/main`) replaces `KBUILD_MODNAME` (e.g., `main`), making module names unique. Backward compatibility is preserved by also matching against `kbasename(di->mod_name)`. ```c if (query->module && !match_wildcard_hyphen(query->module, di->mod_name) && !match_wildcard_hyphen(query->module, kbasename(di->mod_name))) continue; ``` This is the right approach -- `module main` still works (matching all 4 `main` modules), but now `module init/main` can target one specifically. **Minor concern:** The `#ifdef KBUILD_MODFILE` guard is checking whether the macro is defined, not whether the module is builtin. For loadable modules, `KBUILD_MODFILE` may also be defined -- this should be verified. Looking at the Kbuild system, `KBUILD_MODFILE` is defined for both builtins and modules, but for modules the path-based name is still unique, so this shouldn't cause problems in practice. However, the commit message claims "KBUILD_MODNAME for loadables" -- if KBUILD_MODFILE is defined for modules too, the actual behavior is KBUILD_MODFILE for both, which is still correct (module names remain unique either way) but the commit message is slightly misleading. The selftest additions (`test_subsystem_module_queries` and `test_hyphen_underscore`) provide good coverage. **Nit in the cover letter:** The changelog still has `EDITME` placeholders: ``` - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. ``` This is below the `---` so it won't appear in the final commit, but it looks sloppy in the mail posting. --- Generated by Claude Code Patch Reviewer