2019-01-29 20:44:33

by Jacob Keller

[permalink] [raw]
Subject: [PATCH] namespace: fix namespace.pl script to support relative paths

The namespace.pl script does not work properly if objtree is not set to
an absolute path. The do_nm function is run from within the find
function, which changes directories.

Because of this, appending objtree, $File::Find::dir, and $source, will
return a path which is not valid from the current directory.

This used to work when objtree was set to an absolute path when using
"make namespacecheck". It appears to have not worked when calling
./scripts/namespace.pl directly.

This behavior was changed in 7e1c04779efd ("kbuild: Use relative path
for $(objtree)", 2014-05-14)

Rather than fixing the Makefile to set objtree to an absolute path, just
fix namespace.pl to work when srctree and objtree are relative. Also fix
the script to use an absolute path for these by default.

Use the File::Spec module for this purpose. It's been part of perl
5 since 5.005.

The curdir() function is used to get the current directory when the
objtree and srctree aren't set in the environment.

rel2abs() is used to convert possibly relative objtree and srctree
environment variables to absolute paths.

Finally, the catfile() function is used instead of string appending
paths together, since this is more robust when joining paths together.

Signed-off-by: Jacob Keller <[email protected]>
---
scripts/namespace.pl | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/scripts/namespace.pl b/scripts/namespace.pl
index 6135574a6f39..1da7bca201a4 100755
--- a/scripts/namespace.pl
+++ b/scripts/namespace.pl
@@ -65,13 +65,14 @@
use warnings;
use strict;
use File::Find;
+use File::Spec;

my $nm = ($ENV{'NM'} || "nm") . " -p";
my $objdump = ($ENV{'OBJDUMP'} || "objdump") . " -s -j .comment";
-my $srctree = "";
-my $objtree = "";
-$srctree = "$ENV{'srctree'}/" if (exists($ENV{'srctree'}));
-$objtree = "$ENV{'objtree'}/" if (exists($ENV{'objtree'}));
+my $srctree = File::Spec->curdir();
+my $objtree = File::Spec->curdir();
+$srctree = File::Spec->rel2abs($ENV{'srctree'}) if (exists($ENV{'srctree'}));
+$objtree = File::Spec->rel2abs($ENV{'objtree'}) if (exists($ENV{'objtree'}));

if ($#ARGV != -1) {
print STDERR "usage: $0 takes no parameters\n";
@@ -231,9 +232,9 @@ sub do_nm
}
($source = $basename) =~ s/\.o$//;
if (-e "$source.c" || -e "$source.S") {
- $source = "$objtree$File::Find::dir/$source";
+ $source = File::Spec->catfile($objtree, $File::Find::dir, $source)
} else {
- $source = "$srctree$File::Find::dir/$source";
+ $source = File::Spec->catfile($srctree, $File::Find::dir, $source)
}
if (! -e "$source.c" && ! -e "$source.S") {
# No obvious source, exclude the object if it is conglomerate
--
2.18.0.219.gaf81d287a9da



2019-09-27 23:15:10

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths


re: https://lore.kernel.org/lkml/[email protected]/

Did anything happen with this patch?

Please send it to [email protected] and
Cc: Masahiro Yamada <[email protected]>

You can also add:
Acked-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]>


I was just about to fix this script but I decided to first see if anyone else
had already done so. Thanks.

--
~Randy

2019-09-27 23:24:30

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH] namespace: fix namespace.pl script to support relative paths

> -----Original Message-----
> From: Randy Dunlap [mailto:[email protected]]
> Sent: Friday, September 27, 2019 4:12 PM
> To: Keller, Jacob E <[email protected]>
> Cc: [email protected]; [email protected]; linux-kbuild <linux-
> [email protected]>; Masahiro Yamada <[email protected]>
> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
>
>
> re: https://lore.kernel.org/lkml/[email protected]/
>
> Did anything happen with this patch?
>

I haven't heard anything or seen it get applied.

> Please send it to [email protected] and
> Cc: Masahiro Yamada <[email protected]>
>

Sure, I can forward it.

> You can also add:
> Acked-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]>
>
>
> I was just about to fix this script but I decided to first see if anyone else
> had already done so. Thanks.
>

Thanks for the review/notice here.

> --
> ~Randy

-Jake

2019-09-27 23:31:25

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH] namespace: fix namespace.pl script to support relative paths

> -----Original Message-----
> From: Randy Dunlap [mailto:[email protected]]
> Sent: Friday, September 27, 2019 4:12 PM
> To: Keller, Jacob E <[email protected]>
> Cc: [email protected]; [email protected]; linux-kbuild <linux-
> [email protected]>; Masahiro Yamada <[email protected]>
> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
>
>
> re: https://lore.kernel.org/lkml/[email protected]/
>
> Did anything happen with this patch?
>
> Please send it to [email protected] and
> Cc: Masahiro Yamada <[email protected]>
>
> You can also add:
> Acked-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]>
>
>
> I was just about to fix this script but I decided to first see if anyone else
> had already done so. Thanks.
>
> --
> ~Randy

Done, thanks.

Regards,
Jake

2019-09-29 00:21:51

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths

On Sat, Sep 28, 2019 at 8:30 AM Keller, Jacob E
<[email protected]> wrote:
>
> > -----Original Message-----
> > From: Randy Dunlap [mailto:[email protected]]
> > Sent: Friday, September 27, 2019 4:12 PM
> > To: Keller, Jacob E <[email protected]>
> > Cc: [email protected]; [email protected]; linux-kbuild <linux-
> > [email protected]>; Masahiro Yamada <[email protected]>
> > Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
> >
> >
> > re: https://lore.kernel.org/lkml/[email protected]/
> >
> > Did anything happen with this patch?
> >
> > Please send it to [email protected] and
> > Cc: Masahiro Yamada <[email protected]>
> >
> > You can also add:
> > Acked-by: Randy Dunlap <[email protected]>
> > Tested-by: Randy Dunlap <[email protected]>
> >
> >
> > I was just about to fix this script but I decided to first see if anyone else
> > had already done so. Thanks.
> >
> > --
> > ~Randy
>
> Done, thanks.
>
> Regards,
> Jake


Applied to linux/kbuild. Thanks.

--
Best Regards
Masahiro Yamada

2019-09-30 20:50:54

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH] namespace: fix namespace.pl script to support relative paths

> -----Original Message-----
> From: Masahiro Yamada [mailto:[email protected]]
> Sent: Saturday, September 28, 2019 5:21 PM
> To: Keller, Jacob E <[email protected]>
> Cc: Randy Dunlap <[email protected]>; [email protected]; linux-
> [email protected]; linux-kbuild <[email protected]>
> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
>
> On Sat, Sep 28, 2019 at 8:30 AM Keller, Jacob E
> <[email protected]> wrote:
> >
> > > -----Original Message-----
> > > From: Randy Dunlap [mailto:[email protected]]
> > > Sent: Friday, September 27, 2019 4:12 PM
> > > To: Keller, Jacob E <[email protected]>
> > > Cc: [email protected]; [email protected]; linux-kbuild
> <linux-
> > > [email protected]>; Masahiro Yamada <[email protected]>
> > > Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
> > >
> > >
> > > re: https://lore.kernel.org/lkml/20190129204319.15238-1-
> [email protected]/
> > >
> > > Did anything happen with this patch?
> > >
> > > Please send it to [email protected] and
> > > Cc: Masahiro Yamada <[email protected]>
> > >
> > > You can also add:
> > > Acked-by: Randy Dunlap <[email protected]>
> > > Tested-by: Randy Dunlap <[email protected]>
> > >
> > >
> > > I was just about to fix this script but I decided to first see if anyone else
> > > had already done so. Thanks.
> > >
> > > --
> > > ~Randy
> >
> > Done, thanks.
> >
> > Regards,
> > Jake
>
>
> Applied to linux/kbuild. Thanks.
>

Great, thanks!

> --
> Best Regards
> Masahiro Yamada

2019-10-23 05:47:26

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths

On Tue, Oct 1, 2019 at 5:49 AM Keller, Jacob E <[email protected]> wrote:
>
> > -----Original Message-----
> > From: Masahiro Yamada [mailto:[email protected]]
> > Sent: Saturday, September 28, 2019 5:21 PM
> > To: Keller, Jacob E <[email protected]>
> > Cc: Randy Dunlap <[email protected]>; [email protected]; linux-
> > [email protected]; linux-kbuild <[email protected]>
> > Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
> >
> > On Sat, Sep 28, 2019 at 8:30 AM Keller, Jacob E
> > <[email protected]> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Randy Dunlap [mailto:[email protected]]
> > > > Sent: Friday, September 27, 2019 4:12 PM
> > > > To: Keller, Jacob E <[email protected]>
> > > > Cc: [email protected]; [email protected]; linux-kbuild
> > <linux-
> > > > [email protected]>; Masahiro Yamada <[email protected]>
> > > > Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths
> > > >
> > > >
> > > > re: https://lore.kernel.org/lkml/20190129204319.15238-1-
> > [email protected]/
> > > >
> > > > Did anything happen with this patch?
> > > >
> > > > Please send it to [email protected] and
> > > > Cc: Masahiro Yamada <[email protected]>
> > > >
> > > > You can also add:
> > > > Acked-by: Randy Dunlap <[email protected]>
> > > > Tested-by: Randy Dunlap <[email protected]>
> > > >
> > > >
> > > > I was just about to fix this script but I decided to first see if anyone else
> > > > had already done so. Thanks.
> > > >
> > > > --
> > > > ~Randy
> > >
> > > Done, thanks.
> > >
> > > Regards,
> > > Jake
> >
> >
> > Applied to linux/kbuild. Thanks.
> >
>
> Great, thanks!


This scripts has been 5-year broken,
and I did not see any complaint except from you.
So, I wonder how many people are using this.

Nor, do I understand how to use it.

Could you teach me a bit more about this script?



Something might be missing in my mind, but
I do not know how to use this script in a useful way.



It provides three checks.

[1] list_multiply_defined()

This warns multiple definition of functions.

The compiler would fail if it saw any multiple definition,
so the reports from this check are all false-positive.


[2] resolve_external_references()

This warns unresolved symbols.

The compiler would fail if it saw any unresolved symbol,
so the reports from this check are all false-positive, too.




[3] list_extra_externals

This warns symbols with no reference.

This potentially contains lots of false-positives.
For example, the core framework provides APIs, but if all drivers
are disabled, there is no user of those APIs.




I built the kernel with x86_64_defconfig,
and namespacecheck provides

1400 line reports for [1].
200 line reports for [2].
6800 line reports for [3].

Most of these seem false-positives.



How can I use it for the code improvement?

[3] might be still useful to find 'static' candidates,
but it would be difficult given the amount of the report.

--
Best Regards
Masahiro Yamada

2019-10-24 10:14:40

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH] namespace: fix namespace.pl script to support relative paths

> -----Original Message-----
> From: Masahiro Yamada <[email protected]>
> Sent: Tuesday, October 22, 2019 10:22 PM
> To: Keller, Jacob E <[email protected]>; Randy Dunlap
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-kbuild
> <[email protected]>
> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
> paths
>
> This scripts has been 5-year broken,
> and I did not see any complaint except from you.
> So, I wonder how many people are using this.
>
> Nor, do I understand how to use it.
>
> Could you teach me a bit more about this script?
>
>
>
> Something might be missing in my mind, but
> I do not know how to use this script in a useful way.
>
>
>
> It provides three checks.
>
> [1] list_multiply_defined()
>
> This warns multiple definition of functions.
>
> The compiler would fail if it saw any multiple definition,
> so the reports from this check are all false-positive.
>
>
> [2] resolve_external_references()
>
> This warns unresolved symbols.
>
> The compiler would fail if it saw any unresolved symbol,
> so the reports from this check are all false-positive, too.
>
>

The compiler won't necessarily fail when building modules, because the symbol might be in another loadable module.

>
>
> [3] list_extra_externals
>
> This warns symbols with no reference.
>
> This potentially contains lots of false-positives.
> For example, the core framework provides APIs, but if all drivers
> are disabled, there is no user of those APIs.
>

We use this to help verify that driver modules do not expose symbols.

Thanks,
Jake

2019-10-24 11:03:53

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths

On Thu, Oct 24, 2019 at 6:34 AM Keller, Jacob E
<[email protected]> wrote:
>
> > -----Original Message-----
> > From: Masahiro Yamada <[email protected]>
> > Sent: Tuesday, October 22, 2019 10:22 PM
> > To: Keller, Jacob E <[email protected]>; Randy Dunlap
> > <[email protected]>
> > Cc: [email protected]; [email protected]; linux-kbuild
> > <[email protected]>
> > Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
> > paths
> >
> > This scripts has been 5-year broken,
> > and I did not see any complaint except from you.
> > So, I wonder how many people are using this.
> >
> > Nor, do I understand how to use it.
> >
> > Could you teach me a bit more about this script?
> >
> >
> >
> > Something might be missing in my mind, but
> > I do not know how to use this script in a useful way.
> >
> >
> >
> > It provides three checks.
> >
> > [1] list_multiply_defined()
> >
> > This warns multiple definition of functions.
> >
> > The compiler would fail if it saw any multiple definition,
> > so the reports from this check are all false-positive.
> >
> >
> > [2] resolve_external_references()
> >
> > This warns unresolved symbols.
> >
> > The compiler would fail if it saw any unresolved symbol,
> > so the reports from this check are all false-positive, too.
> >
> >
>
> The compiler won't necessarily fail when building modules, because the symbol might be in another loadable module.

Right, but this is already checked by modpost, isn't it?



> >
> >
> > [3] list_extra_externals
> >
> > This warns symbols with no reference.
> >
> > This potentially contains lots of false-positives.
> > For example, the core framework provides APIs, but if all drivers
> > are disabled, there is no user of those APIs.
> >
>
> We use this to help verify that driver modules do not expose symbols.

Ah, the output is quite large, so
you search for only modules in your interest. Right?


If you want to detect missing 'static',
have you tried 'sparse'?



> Thanks,
> Jake



--
Best Regards
Masahiro Yamada

2019-10-25 20:48:20

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH] namespace: fix namespace.pl script to support relative paths


> -----Original Message-----
> From: Masahiro Yamada <[email protected]>
> Sent: Wednesday, October 23, 2019 6:22 PM
> To: Keller, Jacob E <[email protected]>
> Cc: Randy Dunlap <[email protected]>; [email protected];
> [email protected]; linux-kbuild <[email protected]>
> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
> paths
>
> On Thu, Oct 24, 2019 at 6:34 AM Keller, Jacob E
> <[email protected]> wrote:
> >
> > > -----Original Message-----
> > > From: Masahiro Yamada <[email protected]>
> > > Sent: Tuesday, October 22, 2019 10:22 PM
> > > To: Keller, Jacob E <[email protected]>; Randy Dunlap
> > > <[email protected]>
> > > Cc: [email protected]; [email protected]; linux-
> kbuild
> > > <[email protected]>
> > > Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
> > > paths
> > >
> > > This scripts has been 5-year broken,
> > > and I did not see any complaint except from you.
> > > So, I wonder how many people are using this.
> > >
> > > Nor, do I understand how to use it.
> > >
> > > Could you teach me a bit more about this script?
> > >
> > >
> > >
> > > Something might be missing in my mind, but
> > > I do not know how to use this script in a useful way.
> > >
> > >
> > >
> > > It provides three checks.
> > >
> > > [1] list_multiply_defined()
> > >
> > > This warns multiple definition of functions.
> > >
> > > The compiler would fail if it saw any multiple definition,
> > > so the reports from this check are all false-positive.
> > >
> > >
> > > [2] resolve_external_references()
> > >
> > > This warns unresolved symbols.
> > >
> > > The compiler would fail if it saw any unresolved symbol,
> > > so the reports from this check are all false-positive, too.
> > >
> > >
> >
> > The compiler won't necessarily fail when building modules, because the symbol
> might be in another loadable module.
>
> Right, but this is already checked by modpost, isn't it?
>
>
>
> > >
> > >
> > > [3] list_extra_externals
> > >
> > > This warns symbols with no reference.
> > >
> > > This potentially contains lots of false-positives.
> > > For example, the core framework provides APIs, but if all drivers
> > > are disabled, there is no user of those APIs.
> > >
> >
> > We use this to help verify that driver modules do not expose symbols.
>
> Ah, the output is quite large, so
> you search for only modules in your interest. Right?
>

We run it on only one module at a time, yes.

>
> If you want to detect missing 'static',
> have you tried 'sparse'?
>

We've used that as well.

To be fair, I agree that it covers similar functionality as other tools. I haven't looked directly at namespace.pl output in a while, and the fix here is multiple years old that took a long time to get picked up.

If it's agreed that the tool has no value, and especially if it results in false indications of a problem, then maybe removing it to prevent someone from mis-reading its output makes sense?

Thanks,
Jake

>
>
> > Thanks,
> > Jake
>
>
>
> --
> Best Regards
> Masahiro Yamada

2019-10-25 20:56:10

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative paths

On 10/25/19 10:45 AM, Keller, Jacob E wrote:
>
>> -----Original Message-----
>> From: Masahiro Yamada <[email protected]>
>> Sent: Wednesday, October 23, 2019 6:22 PM
>> To: Keller, Jacob E <[email protected]>
>> Cc: Randy Dunlap <[email protected]>; [email protected];
>> [email protected]; linux-kbuild <[email protected]>
>> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
>> paths
>>
>> On Thu, Oct 24, 2019 at 6:34 AM Keller, Jacob E
>> <[email protected]> wrote:
>>>
>>>> -----Original Message-----
>>>> From: Masahiro Yamada <[email protected]>
>>>> Sent: Tuesday, October 22, 2019 10:22 PM
>>>> To: Keller, Jacob E <[email protected]>; Randy Dunlap
>>>> <[email protected]>
>>>> Cc: [email protected]; [email protected]; linux-
>> kbuild
>>>> <[email protected]>
>>>> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
>>>> paths
>>>>
>>>> This scripts has been 5-year broken,
>>>> and I did not see any complaint except from you.
>>>> So, I wonder how many people are using this.
>>>>
>>>> Nor, do I understand how to use it.
>>>>
>>>> Could you teach me a bit more about this script?
>>>>
>>>>
>>>>
>>>> Something might be missing in my mind, but
>>>> I do not know how to use this script in a useful way.
>>>>
>>>>
>>>>
>>>> It provides three checks.
>>>>
>>>> [1] list_multiply_defined()
>>>>
>>>> This warns multiple definition of functions.
>>>>
>>>> The compiler would fail if it saw any multiple definition,
>>>> so the reports from this check are all false-positive.
>>>>
>>>>
>>>> [2] resolve_external_references()
>>>>
>>>> This warns unresolved symbols.
>>>>
>>>> The compiler would fail if it saw any unresolved symbol,
>>>> so the reports from this check are all false-positive, too.
>>>>
>>>>
>>>
>>> The compiler won't necessarily fail when building modules, because the symbol
>> might be in another loadable module.
>>
>> Right, but this is already checked by modpost, isn't it?
>>
>>
>>
>>>>
>>>>
>>>> [3] list_extra_externals
>>>>
>>>> This warns symbols with no reference.
>>>>
>>>> This potentially contains lots of false-positives.
>>>> For example, the core framework provides APIs, but if all drivers
>>>> are disabled, there is no user of those APIs.
>>>>
>>>
>>> We use this to help verify that driver modules do not expose symbols.
>>
>> Ah, the output is quite large, so
>> you search for only modules in your interest. Right?
>>
>
> We run it on only one module at a time, yes.
>
>>
>> If you want to detect missing 'static',
>> have you tried 'sparse'?
>>
>
> We've used that as well.
>
> To be fair, I agree that it covers similar functionality as other tools. I haven't looked directly at namespace.pl output in a while, and the fix here is multiple years old that took a long time to get picked up.
>
> If it's agreed that the tool has no value, and especially if it results in false indications of a problem, then maybe removing it to prevent someone from mis-reading its output makes sense?

If there is a satisfactory alternative, I expect that namespace.pl is old,
unmaintained, and unneeded, and should go away.

--
~Randy

2019-10-25 20:57:27

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH] namespace: fix namespace.pl script to support relative paths

> -----Original Message-----
> From: Randy Dunlap <[email protected]>
> Sent: Friday, October 25, 2019 12:30 PM
> To: Keller, Jacob E <[email protected]>; Masahiro Yamada
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-kbuild
> <[email protected]>
> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
> paths
>
> On 10/25/19 10:45 AM, Keller, Jacob E wrote:
> >
> >> -----Original Message-----
> >> From: Masahiro Yamada <[email protected]>
> >> Sent: Wednesday, October 23, 2019 6:22 PM
> >> To: Keller, Jacob E <[email protected]>
> >> Cc: Randy Dunlap <[email protected]>; [email protected];
> >> [email protected]; linux-kbuild <[email protected]>
> >> Subject: Re: [PATCH] namespace: fix namespace.pl script to support relative
> >> paths
> >>
> >> If you want to detect missing 'static',
> >> have you tried 'sparse'?
> >>
> >
> > We've used that as well.
> >
> > To be fair, I agree that it covers similar functionality as other tools. I haven't
> looked directly at namespace.pl output in a while, and the fix here is multiple
> years old that took a long time to get picked up.
> >
> > If it's agreed that the tool has no value, and especially if it results in false
> indications of a problem, then maybe removing it to prevent someone from mis-
> reading its output makes sense?
>
> If there is a satisfactory alternative, I expect that namespace.pl is old,
> unmaintained, and unneeded, and should go away.
>
> --
> ~Randy

Given Yamada's comments and my experience, modpost and sparse are both good at detecting the issues that namespace.pl handles.

I am not sure if there's any other output that would be valuable from namespace.pl, but if not.. I don't see a reason to keep it compared to the other tools.

Thanks,
Jake