2004-03-04 10:53:37

by Daniel Mack

[permalink] [raw]
Subject: [PATCH] 2.6.4-rc2: scripts/modpost.c

(This is is a repost - now with a patch for 2.6.4-rc2).

Hi,

as I found out, it's impossible to use the build-chain tool scripts/modpost
of the 2.6 kernel series to externally build modules from a directory that
contains the character sequence '.o'. Weird things happen if you try to do
so.

With a directory structure like on my system here, building the current DVB
driver in '/home/daniel/cvs.linuxtv.org/dvb-kernel/build-2.6i/' generates a
file called '/home/daniel/cvs.linuxtv.mod.c' since modpost cuts every
filename string at the first occurence of '.o', not only the 'trailing .o',
as the comment says.

Here's the patch for 2.6.4-rc2:


--- linux-2.6.4-rc2.orig/scripts/modpost.c 2004-03-04 11:40:21.000000000 +0100
+++ linux-2.6.4-rc2/scripts/modpost.c 2004-03-04 11:23:08.000000000 +0100
@@ -63,16 +63,16 @@
new_module(char *modname)
{
struct module *mod;
- char *p;
+ int len;

mod = NOFAIL(malloc(sizeof(*mod)));
memset(mod, 0, sizeof(*mod));
mod->name = NOFAIL(strdup(modname));

/* strip trailing .o */
- p = strstr(mod->name, ".o");
- if (p)
- *p = 0;
+ len = strlen(mod->name);
+ if (len > 2 && mod->name[len-2] == '.' && mod->name[len-1] == 'o')
+ mod->name[len-2] = 0;

/* add to list */
mod->next = modules;



Daniel


2004-03-04 11:54:42

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c

Daniel Mack <[email protected]> wrote:
>
> as I found out, it's impossible to use the build-chain tool scripts/modpost
> of the 2.6 kernel series to externally build modules from a directory that
> contains the character sequence '.o'. Weird things happen if you try to do
> so.
>
> With a directory structure like on my system here, building the current DVB
> driver in '/home/daniel/cvs.linuxtv.org/dvb-kernel/build-2.6i/' generates a
> file called '/home/daniel/cvs.linuxtv.mod.c' since modpost cuts every
> filename string at the first occurence of '.o', not only the 'trailing .o',
> as the comment says.

I tried your patch the other day and had weird compilation errors with x86
allyesconfig. Could you check that?

2004-03-04 12:10:33

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c

On Thu, Mar 04, 2004 at 03:54:27AM -0800, Andrew Morton wrote:
> I tried your patch the other day and had weird compilation errors with x86
> allyesconfig. Could you check that?

I tried this patch on several hosts. Both the modpost tool and all modules
were built fine. What compilation errors did you encounter?

Daniel

2004-03-05 05:29:00

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c


| (This is is a repost - now with a patch for 2.6.4-rc2).
|
| Hi,
|
| as I found out, it's impossible to use the build-chain tool scripts/modpost
| of the 2.6 kernel series to externally build modules from a directory that
| contains the character sequence '.o'. Weird things happen if you try to do
| so.
|
| With a directory structure like on my system here, building the current DVB
| driver in '/home/daniel/cvs.linuxtv.org/dvb-kernel/build-2.6i/' generates a
| file called '/home/daniel/cvs.linuxtv.mod.c' since modpost cuts every
| filename string at the first occurence of '.o', not only the 'trailing .o',
| as the comment says.

The comment and code certainly don't match, and your patch makes sense
to me. However, I can't reproduce the problem that you describe.

I built the kernel image and modules in "http://www.osdl.org/264rc2/build1",
and all *.mod.c and *.ko ended up there with no problems.
Then I modified modpost.c (from 2.6.4-rc1, without your patch) to
print the "stripped" module names (without the trailing ".o")
and saw a list like this:
modpost: stripped mod.name=[fs/jfs/jfs]

so where are the parent directory names that are causing problems
for you coming from?


Andrew, I applied the patch and didn't have any problems with
'make allyesconfig' like you alluded to.


| Here's the patch for 2.6.4-rc2:
|
|
| --- linux-2.6.4-rc2.orig/scripts/modpost.c 2004-03-04 11:40:21.000000000 +0100
| +++ linux-2.6.4-rc2/scripts/modpost.c 2004-03-04 11:23:08.000000000 +0100
| @@ -63,16 +63,16 @@
| new_module(char *modname)
| {
| struct module *mod;
| - char *p;
| + int len;
|
| mod = NOFAIL(malloc(sizeof(*mod)));
| memset(mod, 0, sizeof(*mod));
| mod->name = NOFAIL(strdup(modname));
|
| /* strip trailing .o */
| - p = strstr(mod->name, ".o");
| - if (p)
| - *p = 0;
| + len = strlen(mod->name);
| + if (len > 2 && mod->name[len-2] == '.' && mod->name[len-1] == 'o')
| + mod->name[len-2] = 0;
|
| /* add to list */
| mod->next = modules;

--
~Randy

2004-03-05 06:22:11

by Tony Breeds

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c

On Thu, Mar 04, 2004 at 09:24:40PM -0800, Randy.Dunlap wrote:

> The comment and code certainly don't match, and your patch makes sense
> to me. However, I can't reproduce the problem that you describe.
>
> I built the kernel image and modules in "http://www.osdl.org/264rc2/build1",
> and all *.mod.c and *.ko ended up there with no problems.
> Then I modified modpost.c (from 2.6.4-rc1, without your patch) to
> print the "stripped" module names (without the trailing ".o")
> and saw a list like this:
> modpost: stripped mod.name=[fs/jfs/jfs]
>
> so where are the parent directory names that are causing problems
> for you coming from?

When building external modules.

Yours Tony

linux.conf.au http://lca2005.linux.org.au/
Apr 18-23 2005 The Australian Linux Technical Conference!

2004-03-05 11:48:30

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c

On Thu, Mar 04, 2004 at 09:24:40PM -0800, Randy.Dunlap wrote:
> The comment and code certainly don't match, and your patch makes sense
> to me. However, I can't reproduce the problem that you describe.
>
> I built the kernel image and modules in "http://www.osdl.org/264rc2/build1",
> and all *.mod.c and *.ko ended up there with no problems.

This only occurs if you try to build kernel modules externally like
described in http://linuxdevices.com/articles/AT4389927951.html.
The DVB driver from CVS does it like this, I'm sure there are many
more cases where this could lead to problems.

> Andrew, I applied the patch and didn't have any problems with
> 'make allyesconfig' like you alluded to.

Hmm, here is a compiler warning when builing modpost since my patch
accesses const char* memory directly (right after malloc()ing it).
The following patch has exactly the same effect but also makes gcc
happy.


diff -ru linux-2.6.4-rc2.orig/scripts/modpost.c linux-2.6.4-rc2/scripts/modpost.c
--- linux-2.6.4-rc2.orig/scripts/modpost.c 2004-03-04 11:40:21.000000000 +0100
+++ linux-2.6.4-rc2/scripts/modpost.c 2004-03-05 12:10:24.000000000 +0100
@@ -63,17 +63,18 @@
new_module(char *modname)
{
struct module *mod;
- char *p;
+ int len;

mod = NOFAIL(malloc(sizeof(*mod)));
memset(mod, 0, sizeof(*mod));
- mod->name = NOFAIL(strdup(modname));
-
+ len = strlen(modname);
+
/* strip trailing .o */
- p = strstr(mod->name, ".o");
- if (p)
- *p = 0;
-
+ if (len > 2 && modname[len-2] == '.' && modname[len-1] == 'o')
+ len -= 2;
+
+ mod->name = NOFAIL(strndup(modname, len));
+
/* add to list */
mod->next = modules;
modules = mod;
diff -ru linux-2.6.4-rc2.orig/scripts/modpost.h linux-2.6.4-rc2/scripts/modpost.h
--- linux-2.6.4-rc2.orig/scripts/modpost.h 2004-03-04 11:40:21.000000000 +0100
+++ linux-2.6.4-rc2/scripts/modpost.h 2004-03-05 12:10:51.000000000 +0100
@@ -1,3 +1,5 @@
+#define _GNU_SOURCE
+
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>


Daniel

2004-03-11 00:41:22

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c

On Thu, 2004-03-04 at 22:37, Daniel Mack wrote:

> --- linux-2.6.4-rc2.orig/scripts/modpost.c 2004-03-04 11:40:21.000000000 +0100
> +++ linux-2.6.4-rc2/scripts/modpost.c 2004-03-04 11:23:08.000000000 +0100
> @@ -63,16 +63,16 @@
> new_module(char *modname)
> {
> struct module *mod;
> - char *p;
> + int len;
>
> mod = NOFAIL(malloc(sizeof(*mod)));
> memset(mod, 0, sizeof(*mod));
> mod->name = NOFAIL(strdup(modname));
>
> /* strip trailing .o */
> - p = strstr(mod->name, ".o");
> - if (p)
> - *p = 0;
> + len = strlen(mod->name);
> + if (len > 2 && mod->name[len-2] == '.' && mod->name[len-1] == 'o')
> + mod->name[len-2] = 0;
>
> /* add to list */
> mod->next = modules;

Please use strrchr(mod->name, '.'). More readable, simpler, and ever
arguably more correct.

Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell

2004-03-11 05:51:27

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] 2.6.4-rc2: scripts/modpost.c

On Thu, Mar 11, 2004 at 11:40:18AM +1100, Rusty Russell wrote:
> On Thu, 2004-03-04 at 22:37, Daniel Mack wrote:
>
> > --- linux-2.6.4-rc2.orig/scripts/modpost.c 2004-03-04 11:40:21.000000000 +0100
> > +++ linux-2.6.4-rc2/scripts/modpost.c 2004-03-04 11:23:08.000000000 +0100
> > @@ -63,16 +63,16 @@
> > new_module(char *modname)
> > {
> > struct module *mod;
> > - char *p;
> > + int len;
> >
> > mod = NOFAIL(malloc(sizeof(*mod)));
> > memset(mod, 0, sizeof(*mod));
> > mod->name = NOFAIL(strdup(modname));
> >
> > /* strip trailing .o */
> > - p = strstr(mod->name, ".o");
> > - if (p)
> > - *p = 0;
> > + len = strlen(mod->name);
> > + if (len > 2 && mod->name[len-2] == '.' && mod->name[len-1] == 'o')
> > + mod->name[len-2] = 0;
> >
> > /* add to list */
> > mod->next = modules;
>
> Please use strrchr(mod->name, '.'). More readable, simpler, and ever
> arguably more correct.
OK.
I have some pending modpost changes (to fix MODULE_VERSION with O=),
so I will include the fix there.

Sam