2010-01-19 06:51:54

by Mike Frysinger

[permalink] [raw]
Subject: [PATCH] kconfig: dont hardcode path to lsmod

The lsmod utility has always been installed into /bin with the newer
module-init-tools package, so let lsmod be found via PATH instead of
hardcoding the old modutils /sbin path.

Signed-off-by: Mike Frysinger <[email protected]>
---
scripts/kconfig/streamline_config.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 0d80082..e1dfc8c 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -238,7 +238,7 @@ foreach my $makefile (@makefiles) {
my %modules;

# see what modules are loaded on this system
-open(LIN,"/sbin/lsmod|") || die "Cant lsmod";
+open(LIN,"lsmod|") || die "Cant lsmod";
while (<LIN>) {
next if (/^Module/); # Skip the first line.
if (/^(\S+)/) {
--
1.6.6


2010-01-19 14:10:19

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, 2010-01-19 at 01:52 -0500, Mike Frysinger wrote:
> The lsmod utility has always been installed into /bin with the newer
> module-init-tools package, so let lsmod be found via PATH instead of
> hardcoding the old modutils /sbin path.
>
> Signed-off-by: Mike Frysinger <[email protected]>

Thanks Mike, I actually have another patch like this. I was first a bit
nervous about using non hard code, but then perhaps it's OK. I my patch
actually checked the main paths first before choosing the "lsmod". But
maybe that would be the better solution, since it would allow someone to
choose their own lsmod.

Thanks!

-- Steve

> ---
> scripts/kconfig/streamline_config.pl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 0d80082..e1dfc8c 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -238,7 +238,7 @@ foreach my $makefile (@makefiles) {
> my %modules;
>
> # see what modules are loaded on this system
> -open(LIN,"/sbin/lsmod|") || die "Cant lsmod";
> +open(LIN,"lsmod|") || die "Cant lsmod";
> while (<LIN>) {
> next if (/^Module/); # Skip the first line.
> if (/^(\S+)/) {

2010-01-19 14:24:07

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>The lsmod utility has always been installed into /bin with the newer
>module-init-tools package, so let lsmod be found via PATH instead of
>hardcoding the old modutils /sbin path.
>

Some distro doesn't set /sbin to PATH, so for me a better solution
would be making PATH contain /sbin, and then use "lsmod".

Thanks.

--
Live like a child, think like the god.

2010-01-19 16:31:11

by John Kacur

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod


>> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>> The lsmod utility has always been installed into /bin with the newer
>> module-init-tools package, so let lsmod be found via PATH instead of
>> hardcoding the old modutils /sbin path.
>>
>
> Some distro doesn't set /sbin to PATH, so for me a better solution
> would be making PATH contain /sbin, and then use "lsmod".

How about the following solution then? (Warning, untested)

>From 6a98295f6dc260d13e1abb39210a2a832c9bdd1f Mon Sep 17 00:00:00 2001
From: John Kacur <[email protected]>
Date: Tue, 19 Jan 2010 17:10:48 +0100
Subject: [PATCH] kconfig: If lsmod is not in the /sbin, check the path
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Mike Frysinger reported that lsmod is installed in /bin on newer kernels
which causes a problem when we hardcode the path to /sbin

However, Am?rico Wang reports that some distros don't have /sbin in PATH,
so you can't just let lsmod be found via PATH.

So, first check if lsmod is at /sbin/lsmod, and then check PATH if that fails.

Signed-off-by: John Kacur <[email protected]>
---
scripts/kconfig/streamline_config.pl | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 0d80082..1803d2e 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -238,7 +238,8 @@ foreach my $makefile (@makefiles) {
my %modules;

# see what modules are loaded on this system
-open(LIN,"/sbin/lsmod|") || die "Cant lsmod";
+# If lsmod isn't in the sbin dir, check if it is in the path
+open(LIN,"/sbin/lsmod|") || open(LIN,"lsmod|") || die "Cant lsmod";
while (<LIN>) {
next if (/^Module/); # Skip the first line.
if (/^(\S+)/) {
--
1.6.0.6

2010-01-19 16:38:28

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, 2010-01-19 at 17:29 +0100, John Kacur wrote:

> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 0d80082..1803d2e 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -238,7 +238,8 @@ foreach my $makefile (@makefiles) {
> my %modules;
>
> # see what modules are loaded on this system
> -open(LIN,"/sbin/lsmod|") || die "Cant lsmod";
> +# If lsmod isn't in the sbin dir, check if it is in the path
> +open(LIN,"/sbin/lsmod|") || open(LIN,"lsmod|") || die "Cant lsmod";

I've tried this before, but it gives an error that the "|" pipe failed.

-- Steve


> while (<LIN>) {
> next if (/^Module/); # Skip the first line.
> if (/^(\S+)/) {

2010-01-19 17:23:40

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, Jan 19, 2010 at 09:25, Américo Wang wrote:
> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>>The lsmod utility has always been installed into /bin with the newer
>>module-init-tools package, so let lsmod be found via PATH instead of
>>hardcoding the old modutils /sbin path.
>>
>
> Some distro doesn't set /sbin to PATH, so for me a better solution
> would be making PATH contain /sbin, and then use "lsmod".

read my changelog -- module-init-tools has always installed into /bin.
so what your distro does with /sbin doesnt matter.
-mike

2010-01-19 17:38:38

by John Kacur

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod



On Tue, 19 Jan 2010, Steven Rostedt wrote:

> On Tue, 2010-01-19 at 17:29 +0100, John Kacur wrote:
>
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index 0d80082..1803d2e 100644
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -238,7 +238,8 @@ foreach my $makefile (@makefiles) {
> > my %modules;
> >
> > # see what modules are loaded on this system
> > -open(LIN,"/sbin/lsmod|") || die "Cant lsmod";
> > +# If lsmod isn't in the sbin dir, check if it is in the path
> > +open(LIN,"/sbin/lsmod|") || open(LIN,"lsmod|") || die "Cant lsmod";
>
> I've tried this before, but it gives an error that the "|" pipe failed.
>
> -- Steve
>
>
> > while (<LIN>) {
> > next if (/^Module/); # Skip the first line.
> > if (/^(\S+)/) {
>
>

Are you sure? It works for my test toy program.
First tested with the hardcoded path, and then giving a nonsense name
instead of sbin so that it uses the PATH variable.

John

2010-01-19 17:39:16

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, 2010-01-19 at 12:23 -0500, Mike Frysinger wrote:
> On Tue, Jan 19, 2010 at 09:25, Américo Wang wrote:
> > On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
> >>The lsmod utility has always been installed into /bin with the newer
> >>module-init-tools package, so let lsmod be found via PATH instead of
> >>hardcoding the old modutils /sbin path.
> >>
> >
> > Some distro doesn't set /sbin to PATH, so for me a better solution
> > would be making PATH contain /sbin, and then use "lsmod".
>
> read my changelog -- module-init-tools has always installed into /bin.
> so what your distro does with /sbin doesnt matter.

I still have boxes where lsmod is in /sbin, but that's beside the point.

The patch I made checked for lsmod
in /sbin /bin /usr/bin /usr/sbin /usr/local/bin and /usr/local/sbin. I
could also make it do "lsmod" after that.

-- Steve

2010-01-19 17:42:17

by John Kacur

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, Jan 19, 2010 at 6:23 PM, Mike Frysinger <[email protected]> wrote:
> On Tue, Jan 19, 2010 at 09:25, Am?rico Wang wrote:
>> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>>>The lsmod utility has always been installed into /bin with the newer
>>>module-init-tools package, so let lsmod be found via PATH instead of
>>>hardcoding the old modutils /sbin path.
>>>
>>
>> Some distro doesn't set /sbin to PATH, so for me a better solution
>> would be making PATH contain /sbin, and then use "lsmod".
>
> read my changelog -- module-init-tools has always installed into /bin.
> ?so what your distro does with /sbin doesnt matter.
> -mike
> --

I prefer my patches work for the real-world instead of the "so what
your distro does doesn't matter" world.

2010-01-19 17:55:22

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, Jan 19, 2010 at 12:42, John Kacur wrote:
> On Tue, Jan 19, 2010 at 6:23 PM, Mike Frysinger wrote:
>> On Tue, Jan 19, 2010 at 09:25, Américo Wang wrote:
>>> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>>>>The lsmod utility has always been installed into /bin with the newer
>>>>module-init-tools package, so let lsmod be found via PATH instead of
>>>>hardcoding the old modutils /sbin path.
>>>>
>>>
>>> Some distro doesn't set /sbin to PATH, so for me a better solution
>>> would be making PATH contain /sbin, and then use "lsmod".
>>
>> read my changelog -- module-init-tools has always installed into /bin.
>>  so what your distro does with /sbin doesnt matter.
>
> I prefer my patches work for the real-world instead of the "so what
> your distro does doesn't matter" world.

try reading my comment instead of getting huffy. if you have a distro
that does something stupid like break the correct default m-i-t
install setup, you should actually point it out. the ones i checked
were sane and installed lsmod into /bin (and some symlinked lsmod for
backwards compat with modutils into /sbin).
-mike

2010-01-19 18:12:50

by John Kacur

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod



On Tue, 19 Jan 2010, Mike Frysinger wrote:

> On Tue, Jan 19, 2010 at 12:42, John Kacur wrote:
> > On Tue, Jan 19, 2010 at 6:23 PM, Mike Frysinger wrote:
> >> On Tue, Jan 19, 2010 at 09:25, Américo Wang wrote:
> >>> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
> >>>>The lsmod utility has always been installed into /bin with the newer
> >>>>module-init-tools package, so let lsmod be found via PATH instead of
> >>>>hardcoding the old modutils /sbin path.
> >>>>
> >>>
> >>> Some distro doesn't set /sbin to PATH, so for me a better solution
> >>> would be making PATH contain /sbin, and then use "lsmod".
> >>
> >> read my changelog -- module-init-tools has always installed into /bin.
> >>  so what your distro does with /sbin doesnt matter.
> >
> > I prefer my patches work for the real-world instead of the "so what
> > your distro does doesn't matter" world.
>
> try reading my comment instead of getting huffy. if you have a distro
> that does something stupid like break the correct default m-i-t
> install setup, you should actually point it out. the ones i checked
> were sane and installed lsmod into /bin (and some symlinked lsmod for
> backwards compat with modutils into /sbin).
> -mike

Well, I'm currently running Fedora (10 thru 12), and lsmod is in /sbin
Your patch would still not break for me because /sbin is in the PATH.

However if Américo is correct that there are distros that have lsmod in
/sbin and don't have /sbin in the PATH, then your patch would break them.
You can argue that the distro is doing something stupid, but I'll bet you
they will blame your patch for breaking them. It seems reasonable to
me that a distro might only put /sbin in the superuser path, so I can
imagine there are cases like Américo suggests.

Thanks.

2010-01-19 19:19:09

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Tue, Jan 19, 2010 at 13:12, John Kacur wrote:
> On Tue, 19 Jan 2010, Mike Frysinger wrote:
>> On Tue, Jan 19, 2010 at 12:42, John Kacur wrote:
>> > On Tue, Jan 19, 2010 at 6:23 PM, Mike Frysinger wrote:
>> >> On Tue, Jan 19, 2010 at 09:25, Américo Wang wrote:
>> >>> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>> >>>>The lsmod utility has always been installed into /bin with the newer
>> >>>>module-init-tools package, so let lsmod be found via PATH instead of
>> >>>>hardcoding the old modutils /sbin path.
>> >>>>
>> >>>
>> >>> Some distro doesn't set /sbin to PATH, so for me a better solution
>> >>> would be making PATH contain /sbin, and then use "lsmod".
>> >>
>> >> read my changelog -- module-init-tools has always installed into /bin.
>> >>  so what your distro does with /sbin doesnt matter.
>> >
>> > I prefer my patches work for the real-world instead of the "so what
>> > your distro does doesn't matter" world.
>>
>> try reading my comment instead of getting huffy.  if you have a distro
>> that does something stupid like break the correct default m-i-t
>> install setup, you should actually point it out.  the ones i checked
>> were sane and installed lsmod into /bin (and some symlinked lsmod for
>> backwards compat with modutils into /sbin).
>
> Well, I'm currently running Fedora (10 thru 12), and lsmod is in /sbin
> Your patch would still not break for me because /sbin is in the PATH.
>
> However if Américo is correct that there are distros that have lsmod in
> /sbin and don't have /sbin in the PATH, then your patch would break them.
> You can argue that the distro is doing something stupid, but I'll bet you
> they will blame your patch for breaking them. It seems reasonable to
> me that a distro might only put /sbin in the superuser path, so I can
> imagine there are cases like Américo suggests.

i am saying they're stupid for doing this, but i'm not saying we
shouldnt support it. the premise for my original patch was that
upstream m-i-t has always used /bin (which means all 2.6 module
handers should be there per upstream), and the downstream distros i
had access to followed upstream's direction.

sounds like Fedora should have a bug report to get their things fixed
... there's no reason for `lsmod` to not be in /bin (and everyone's
PATH) since it only reads /proc/modules and that is word readable.
-mike

2010-01-20 03:16:22

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH] kconfig: dont hardcode path to lsmod

On Wed, Jan 20, 2010 at 3:18 AM, Mike Frysinger <[email protected]> wrote:
> On Tue, Jan 19, 2010 at 13:12, John Kacur wrote:
>> On Tue, 19 Jan 2010, Mike Frysinger wrote:
>>> On Tue, Jan 19, 2010 at 12:42, John Kacur wrote:
>>> > On Tue, Jan 19, 2010 at 6:23 PM, Mike Frysinger wrote:
>>> >> On Tue, Jan 19, 2010 at 09:25, Américo Wang wrote:
>>> >>> On Tue, Jan 19, 2010 at 01:52:00AM -0500, Mike Frysinger wrote:
>>> >>>>The lsmod utility has always been installed into /bin with the newer
>>> >>>>module-init-tools package, so let lsmod be found via PATH instead of
>>> >>>>hardcoding the old modutils /sbin path.
>>> >>>>
>>> >>>
>>> >>> Some distro doesn't set /sbin to PATH, so for me a better solution
>>> >>> would be making PATH contain /sbin, and then use "lsmod".
>>> >>
>>> >> read my changelog -- module-init-tools has always installed into /bin.
>>> >>  so what your distro does with /sbin doesnt matter.
>>> >
>>> > I prefer my patches work for the real-world instead of the "so what
>>> > your distro does doesn't matter" world.
>>>
>>> try reading my comment instead of getting huffy.  if you have a distro
>>> that does something stupid like break the correct default m-i-t
>>> install setup, you should actually point it out.  the ones i checked
>>> were sane and installed lsmod into /bin (and some symlinked lsmod for
>>> backwards compat with modutils into /sbin).
>>
>> Well, I'm currently running Fedora (10 thru 12), and lsmod is in /sbin
>> Your patch would still not break for me because /sbin is in the PATH.
>>
>> However if Américo is correct that there are distros that have lsmod in
>> /sbin and don't have /sbin in the PATH, then your patch would break them.
>> You can argue that the distro is doing something stupid, but I'll bet you
>> they will blame your patch for breaking them. It seems reasonable to
>> me that a distro might only put /sbin in the superuser path, so I can
>> imagine there are cases like Américo suggests.
>
> i am saying they're stupid for doing this, but i'm not saying we
> shouldnt support it.  the premise for my original patch was that
> upstream m-i-t has always used /bin (which means all 2.6 module
> handers should be there per upstream), and the downstream distros i
> had access to followed upstream's direction.
>
> sounds like Fedora should have a bug report to get their things fixed
> ... there's no reason for `lsmod` to not be in /bin (and everyone's
> PATH) since it only reads /proc/modules and that is word readable.

This certainly makes sense, but the fact is that lsmod is still there,
what is worse, on RHEL (Fedora too, I think) /sbin is not in PATH
of my zsh.