| Message ID | 20241015231925.3854230-2-mmaurer@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Extended MODVERSIONS Support | expand |
Hi, On Tue, Oct 15, 2024 at 4:19 PM Matthew Maurer <mmaurer@google.com> wrote: > > The `export_report.pl` script was broken [1] a while back due to a code > cleanup causing the regex to no longer match. Additionally, it assumes a > `modules.order` file containing `.ko` in a build directory with `.mod.c` > files. I cannot find when this would have been the case in the history, > as normally `.ko` files only appear in `modules.order` in installed > modules directories, and those do not contain `.mod.c` files. > This patch makes it able to report symbol usage counts for a build tree > with modules and MODVERSIONS. > > Since the rest of this series will change the format of `.mod.c`, this > change fixes the script to work correctly against a current build tree. > Given that the regex no longer matches the format used in `.mod.c`, it > cannot have worked since 2019, so updating this script is purely out of > an abundance of caution. I am unsure who uses this script or for what > purpose. > > * modules.order in a build directory uses .o, not .ko files. Allow .o > files when parsing modules.order. > * The .mod.c format changed [1] how it expressed the section attribute, > leading to a regex mismatch. Update it to match modpost.c > > [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/ If this script has been broken for half a decade and nobody noticed, does anyone actually use it? If this is dead code, I would prefer to just delete it. Sami
On Wed, Oct 16, 2024 at 05:29:21PM -0700, Sami Tolvanen wrote: > Hi, > > On Tue, Oct 15, 2024 at 4:19 PM Matthew Maurer <mmaurer@google.com> wrote: > > > > The `export_report.pl` script was broken [1] a while back due to a code > > cleanup causing the regex to no longer match. Additionally, it assumes a > > `modules.order` file containing `.ko` in a build directory with `.mod.c` > > files. I cannot find when this would have been the case in the history, > > as normally `.ko` files only appear in `modules.order` in installed > > modules directories, and those do not contain `.mod.c` files. > > This patch makes it able to report symbol usage counts for a build tree > > with modules and MODVERSIONS. > > > > Since the rest of this series will change the format of `.mod.c`, this > > change fixes the script to work correctly against a current build tree. > > Given that the regex no longer matches the format used in `.mod.c`, it > > cannot have worked since 2019, so updating this script is purely out of > > an abundance of caution. I am unsure who uses this script or for what > > purpose. > > > > * modules.order in a build directory uses .o, not .ko files. Allow .o > > files when parsing modules.order. > > * The .mod.c format changed [1] how it expressed the section attribute, > > leading to a regex mismatch. Update it to match modpost.c > > > > [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/ > > If this script has been broken for half a decade and nobody noticed, > does anyone actually use it? If this is dead code, I would prefer to > just delete it. I'm in full agreement, please trace back the history down to why the heck we have this, otherwise chuck it. Luis
On Wed, Oct 16, 2024 at 1:19 AM Matthew Maurer <mmaurer@google.com> wrote: > > The `export_report.pl` script was broken [1] a while back due to a code > cleanup causing the regex to no longer match. Instead of the link to lore, you can refer to commit a3d0cb04f7df ("modpost: use __section in the output to *.mod.c") > Additionally, it assumes a > `modules.order` file containing `.ko` in a build directory with `.mod.c` > files. I cannot find when this would have been the case in the history, > as normally `.ko` files only appear in `modules.order` in installed > modules directories, and those do not contain `.mod.c` files. If necessary, you can refer to commit f65a486821cf ("kbuild: change module.order to list *.o instead of *.ko") As suggested, I vote for the removal since it has been broken for 5 years since a3d0cb04f7df.
diff --git a/scripts/export_report.pl b/scripts/export_report.pl index feb3d5542a62..30b5f7819086 100755 --- a/scripts/export_report.pl +++ b/scripts/export_report.pl @@ -55,6 +55,7 @@ sub collectcfiles { open my $fh, '< modules.order' or die "cannot open modules.order: $!\n"; while (<$fh>) { s/\.ko$/.mod.c/; + s/\.o$/.mod.c/; push (@file, $_) } close($fh); @@ -120,7 +121,7 @@ foreach my $thismod (@allcfiles) { next; } if ($state == 1) { - $state = 2 if ($_ =~ /__attribute__\(\(section\("__versions"\)\)\)/); + $state = 2 if ($_ =~ /__used __section\("__versions"\)/); next; } if ($state == 2) {
The `export_report.pl` script was broken [1] a while back due to a code cleanup causing the regex to no longer match. Additionally, it assumes a `modules.order` file containing `.ko` in a build directory with `.mod.c` files. I cannot find when this would have been the case in the history, as normally `.ko` files only appear in `modules.order` in installed modules directories, and those do not contain `.mod.c` files. This patch makes it able to report symbol usage counts for a build tree with modules and MODVERSIONS. Since the rest of this series will change the format of `.mod.c`, this change fixes the script to work correctly against a current build tree. Given that the regex no longer matches the format used in `.mod.c`, it cannot have worked since 2019, so updating this script is purely out of an abundance of caution. I am unsure who uses this script or for what purpose. * modules.order in a build directory uses .o, not .ko files. Allow .o files when parsing modules.order. * The .mod.c format changed [1] how it expressed the section attribute, leading to a regex mismatch. Update it to match modpost.c [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/ Signed-off-by: Matthew Maurer <mmaurer@google.com> --- scripts/export_report.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)