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

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

[Daniel Mack]

(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

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

[Andrew Morton]

Daniel Mack <daniel@zonque.org> 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?

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

[Daniel Mack]

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

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

[Randy.Dunlap]

| (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 "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

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

[Tony Breeds]

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 "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!

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

[Daniel Mack]

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 "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

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

[Rusty Russell]

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

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

[Sam Ravnborg]

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