[PATCH][RFC] disable built-in modules V2

[PATCH][RFC] disable built-in modules V2

[Magnus Damm]

Here comes version 2 of the disable built-in patch.

This patch makes it possible to disable built-in code from the kernel
command line. The patch is rather simple - it extends the compiled-in case 
of module_init() to include __setup() with a name based on KBUILD_MODNAME.

As an example, if you want to disable built-in firewire support then you
could pass "force_ohci1394=off" on the kernel command line. The feature added
by this patch comes handy when you want to boot a precompiled kernel from a 
live cd on som exotic hardware that makes built-in drivers crash.

Changes since last release:
- Upgraded and tested with 2.6.12-rc2
- Added "force_"-prefix to cope with namespace pollution

Signed-off-by: Magnus Damm <damm@opensource.se>

diff -urNp linux-2.6.12-rc2/include/linux/init.h linux-2.6.12-rc2-disable_builtin/include/linux/init.h
--- linux-2.6.12-rc2/include/linux/init.h	2005-04-05 16:57:29.000000000 +0200
+++ linux-2.6.12-rc2-disable_builtin/include/linux/init.h	2005-04-06 00:26:07.000000000 +0200
@@ -143,6 +143,16 @@ struct obs_kernel_param {
 
 /* Relies on saved_command_line being set */
 void __init parse_early_param(void);
+
+void __init disable_initcall(void *fn);
+#define __module_init_disable(x)				\
+static int __init x##_disable_module(char *str)			\
+{								\
+	disable_initcall(x);					\
+	return 1;						\
+}								\
+__setup("force_" __stringify(KBUILD_MODNAME) "=off", x##_disable_module)
+
 #endif /* __ASSEMBLY__ */
 
 /**
@@ -153,7 +163,7 @@ void __init parse_early_param(void);
  * builtin) or at module insertion time (if a module).  There can only
  * be one per module.
  */
-#define module_init(x)	__initcall(x);
+#define module_init(x)	__initcall(x); __module_init_disable(x);  
 
 /**
  * module_exit() - driver exit entry point
diff -urNp linux-2.6.12-rc2/init/main.c linux-2.6.12-rc2-disable_builtin/init/main.c
--- linux-2.6.12-rc2/init/main.c	2005-04-05 16:59:55.000000000 +0200
+++ linux-2.6.12-rc2-disable_builtin/init/main.c	2005-04-05 21:07:05.000000000 +0200
@@ -539,6 +539,17 @@ struct task_struct *child_reaper = &init
 
 extern initcall_t __initcall_start[], __initcall_end[];
 
+void __init disable_initcall(void *fn)
+{
+	initcall_t *call;
+
+	for (call = __initcall_start; call < __initcall_end; call++) {
+
+		if (*call == fn)
+			*call = NULL;
+	}
+}
+
 static void __init do_initcalls(void)
 {
 	initcall_t *call;
@@ -553,7 +564,8 @@ static void __init do_initcalls(void)
 			printk("\n");
 		}
 
-		(*call)();
+		if (*call)
+			(*call)();
 
 		msg = NULL;
 		if (preempt_count() != count) {

Re: [PATCH][RFC] disable built-in modules V2

[Malcolm Rowe]

Magnus Damm writes:
> Here comes version 2 of the disable built-in patch.

> +void __init disable_initcall(void *fn)
> +{
> +	initcall_t *call;
> +
> +	for (call = __initcall_start; call < __initcall_end; call++) {
> +
> +		if (*call == fn)
> +			*call = NULL;
> +	}
> +}
 

Regardless of anything else, won't this break booting with initcall_debug on 
PPC64/IA64 machines? (see the definition of print_fn_descriptor_symbol() in 
kallsyms.h) 


Regards,
Malcolm

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 6, 2005 12:32 PM, Malcolm Rowe <malcolm-linux@farside.org.uk> wrote:
> Magnus Damm writes:
> > Here comes version 2 of the disable built-in patch.
> 
> > +void __init disable_initcall(void *fn)
> > +{
> > +     initcall_t *call;
> > +
> > +     for (call = __initcall_start; call < __initcall_end; call++) {
> > +
> > +             if (*call == fn)
> > +                     *call = NULL;
> > +     }
> > +}
> 
> Regardless of anything else, won't this break booting with initcall_debug on
> PPC64/IA64 machines? (see the definition of print_fn_descriptor_symbol() in
> kallsyms.h)

Correct, thanks for pointing that out. The code below is probably better:

 static void __init do_initcalls(void)
 {
        initcall_t *call;
@@ -547,6 +558,9 @@ static void __init do_initcalls(void)
        for (call = __initcall_start; call < __initcall_end; call++) {
                char *msg;

+               if (!*call)
+                       continue;
+
                if (initcall_debug) {
                        printk(KERN_DEBUG "Calling initcall 0x%p", *call);
                        print_fn_descriptor_symbol(": %s()", (unsigned
long) *call);

And I guess the idea of replacing the initcall pointer with NULL will
work both with and without function descriptors, right? So we should
be safe on IA64 and PPC64.

Regards,

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Malcolm Rowe]

Magnus Damm writes:
>> Regardless of anything else, won't this break booting with initcall_debug on
>> PPC64/IA64 machines? (see the definition of print_fn_descriptor_symbol() in
>> kallsyms.h)
> 
> Correct, thanks for pointing that out. The code below is probably better: 
> 
>  static void __init do_initcalls(void)
>  {
>         initcall_t *call;
> @@ -547,6 +558,9 @@ static void __init do_initcalls(void)
>         for (call = __initcall_start; call < __initcall_end; call++) {
>                 char *msg; 
> 
> +               if (!*call)
> +                       continue;
> +
>                 if (initcall_debug) {
>                         printk(KERN_DEBUG "Calling initcall 0x%p", *call);
>                         print_fn_descriptor_symbol(": %s()", (unsigned
> long) *call);

Yes, that looks more sensible. It hides the fact that the initcall ever 
existed, rather than explicitly telling you that it's been skipped, but I 
don't imagine that that's ever going to cause a problem in practice (i.e., I 
don't think anyone would ever enable "force_ohci1394=off" by mistake and 
also without noticing). 


> And I guess the idea of replacing the initcall pointer with NULL will
> work both with and without function descriptors, right? So we should
> be safe on IA64 and PPC64.

I think so, though I don't really know a great deal about this area. 


An IA64 descriptor is of the form { &code, &data_context }, and a function 
pointer is a pointer to such a descriptor. Presumably, setting a function 
pointer to NULL will either end up setting the pointer-to-descriptor to NULL 
or the code pointer to NULL, but either way, I would expect the 'if 
(!*call)' comparison to work as intended. 


Best thing would be to get someone on IA64 and/or PPC64 to check this for 
you. Also might be worth checking that the patch works as intended with 
CONFIG_MODULES=n (assuming you haven't already). 


Regards,
Malcolm

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 6, 2005 4:28 PM, Malcolm Rowe <malcolm-linux@farside.org.uk> wrote:
> Magnus Damm writes:
> > And I guess the idea of replacing the initcall pointer with NULL will
> > work both with and without function descriptors, right? So we should
> > be safe on IA64 and PPC64.
> 
> I think so, though I don't really know a great deal about this area.
> 
> An IA64 descriptor is of the form { &code, &data_context }, and a function
> pointer is a pointer to such a descriptor. Presumably, setting a function
> pointer to NULL will either end up setting the pointer-to-descriptor to NULL
> or the code pointer to NULL, but either way, I would expect the 'if
> (!*call)' comparison to work as intended.
> 
> Best thing would be to get someone on IA64 and/or PPC64 to check this for
> you. 

I agree. Do we have any friendly IA64/PPC64 user out there?

> Also might be worth checking that the patch works as intended with
> CONFIG_MODULES=n (assuming you haven't already).

The code works both with CONFIG_MODULES set to "y" and unset.

Thanks,

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Zwane Mwaikambo]

On Thu, 7 Apr 2005, Magnus Damm wrote:

> On Apr 6, 2005 4:28 PM, Malcolm Rowe <malcolm-linux@farside.org.uk> wrote:
> > Magnus Damm writes:
> > > And I guess the idea of replacing the initcall pointer with NULL will
> > > work both with and without function descriptors, right? So we should
> > > be safe on IA64 and PPC64.
> > 
> > I think so, though I don't really know a great deal about this area.
> > 
> > An IA64 descriptor is of the form { &code, &data_context }, and a function
> > pointer is a pointer to such a descriptor. Presumably, setting a function
> > pointer to NULL will either end up setting the pointer-to-descriptor to NULL
> > or the code pointer to NULL, but either way, I would expect the 'if
> > (!*call)' comparison to work as intended.
> > 
> > Best thing would be to get someone on IA64 and/or PPC64 to check this for
> > you. 
> 
> I agree. Do we have any friendly IA64/PPC64 user out there?

In C code it is consistent across all architectures, you only have to 
worry about the descriptors when you do modifications in assembly.

	Zwane

Re: [PATCH][RFC] disable built-in modules V2

[AsterixTheGaul]

Hi,

> -#define module_init(x) __initcall(x);
> +#define module_init(x) __initcall(x); __module_init_disable(x);

It would be better if there is brackets around them... like

#define module_init(x) { __initcall(x); __module_init_disable(x); }

then we know it wont break some code like

if (..)
 module_init(x);

thanks.

Re: [PATCH][RFC] disable built-in modules V2

[Roland Dreier]

 > > -#define module_init(x) __initcall(x);
 > > +#define module_init(x) __initcall(x); __module_init_disable(x);
 > 
 > It would be better if there is brackets around them... like
 >
 > #define module_init(x) { __initcall(x); __module_init_disable(x); }
 >
 > then we know it wont break some code like
 > 
 > if (..)
 >  module_init(x);

This is all completely academic, since module_init() is a declaration
that won't be inside any code, but in general it's better still to use
the do { } while (0) idiom like

#define module_init(x) do { __initcall(x); __module_init_disable(x); } while (0)

so it won't break code like

	if (..)
		module_init(x);
	else
		something_else();

(Yes, that code is nonsense but if you're going to nitpick, go all the way...)

 - R.

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 7, 2005 4:23 AM, Roland Dreier <roland@topspin.com> wrote:
>  > > -#define module_init(x) __initcall(x);
>  > > +#define module_init(x) __initcall(x); __module_init_disable(x);
>  >
>  > It would be better if there is brackets around them... like
>  >
>  > #define module_init(x) { __initcall(x); __module_init_disable(x); }
>  >
>  > then we know it wont break some code like
>  >
>  > if (..)
>  >  module_init(x);
> 
> This is all completely academic, since module_init() is a declaration
> that won't be inside any code, but in general it's better still to use
> the do { } while (0) idiom like
> 
> #define module_init(x) do { __initcall(x); __module_init_disable(x); } while (0)
> 
> so it won't break code like
> 
>         if (..)
>                 module_init(x);
>         else
>                 something_else();
> 
> (Yes, that code is nonsense but if you're going to nitpick, go all the way...)

Right. =)
Anyway, besides nitpicking, is there any reason not to include this
code? Or is the added feature considered plain bloat? Yes, the kernel
will become a bit larger, but all the data added by this patch will go
into the init section.

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Randy.Dunlap]

On Thu, 7 Apr 2005 10:23:32 +0200 Magnus Damm wrote:

| On Apr 7, 2005 4:23 AM, Roland Dreier <roland@topspin.com> wrote:
| >  > > -#define module_init(x) __initcall(x);
| >  > > +#define module_init(x) __initcall(x); __module_init_disable(x);
| >  >
| >  > It would be better if there is brackets around them... like
| >  >
| >  > #define module_init(x) { __initcall(x); __module_init_disable(x); }
| >  >
| >  > then we know it wont break some code like
| >  >
| >  > if (..)
| >  >  module_init(x);
| > 
| > This is all completely academic, since module_init() is a declaration
| > that won't be inside any code, but in general it's better still to use
| > the do { } while (0) idiom like
| > 
| > #define module_init(x) do { __initcall(x); __module_init_disable(x); } while (0)
| > 
| > so it won't break code like
| > 
| >         if (..)
| >                 module_init(x);
| >         else
| >                 something_else();
| > 
| > (Yes, that code is nonsense but if you're going to nitpick, go all the way...)
| 
| Right. =)
| Anyway, besides nitpicking, is there any reason not to include this
| code? Or is the added feature considered plain bloat? Yes, the kernel
| will become a bit larger, but all the data added by this patch will go
| into the init section.

Looks like a good idea to me.

---
~Randy

Re: [PATCH][RFC] disable built-in modules V2

[Richard B. Johnson]

On Thu, 7 Apr 2005, Randy.Dunlap wrote:

> On Thu, 7 Apr 2005 10:23:32 +0200 Magnus Damm wrote:
>
> | On Apr 7, 2005 4:23 AM, Roland Dreier <roland@topspin.com> wrote:
> | >  > > -#define module_init(x) __initcall(x);
> | >  > > +#define module_init(x) __initcall(x); __module_init_disable(x);
> | >  >
> | >  > It would be better if there is brackets around them... like
> | >  >
> | >  > #define module_init(x) { __initcall(x); __module_init_disable(x); }
> | >  >
> | >  > then we know it wont break some code like
> | >  >
> | >  > if (..)
> | >  >  module_init(x);
> | >
> | > This is all completely academic, since module_init() is a declaration
> | > that won't be inside any code, but in general it's better still to use
> | > the do { } while (0) idiom like
> | >
> | > #define module_init(x) do { __initcall(x); __module_init_disable(x); } while (0)
> | >
> | > so it won't break code like
> | >
> | >         if (..)
> | >                 module_init(x);
> | >         else
> | >                 something_else();
> | >
> | > (Yes, that code is nonsense but if you're going to nitpick, go all the way...)
> |
> | Right. =)
> | Anyway, besides nitpicking, is there any reason not to include this
> | code? Or is the added feature considered plain bloat? Yes, the kernel
> | will become a bit larger, but all the data added by this patch will go
> | into the init section.
>
> Looks like a good idea to me.
>
> ---
> ~Randy

Can't you disable module-loading with a module? I think so.
You don't need to modify the kernel. Boot-scripts could
just load the "final" module and there is nothing that
can be done to add another module (or even unload existing
ones).

Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

Re: [PATCH][RFC] disable built-in modules V2

[Randy.Dunlap]

On Thu, 7 Apr 2005 13:22:57 -0400 (EDT) Richard B. Johnson wrote:

| On Thu, 7 Apr 2005, Randy.Dunlap wrote:
| 
| > On Thu, 7 Apr 2005 10:23:32 +0200 Magnus Damm wrote:
| >
| > | On Apr 7, 2005 4:23 AM, Roland Dreier <roland@topspin.com> wrote:
| > | >  > > -#define module_init(x) __initcall(x);
| > | >  > > +#define module_init(x) __initcall(x); __module_init_disable(x);
| > | >  >
| > | >  > It would be better if there is brackets around them... like
| > | >  >
| > | >  > #define module_init(x) { __initcall(x); __module_init_disable(x); }
| > | >  >
| > | >  > then we know it wont break some code like
| > | >  >
| > | >  > if (..)
| > | >  >  module_init(x);
| > | >
| > | > This is all completely academic, since module_init() is a declaration
| > | > that won't be inside any code, but in general it's better still to use
| > | > the do { } while (0) idiom like
| > | >
| > | > #define module_init(x) do { __initcall(x); __module_init_disable(x); } while (0)
| > | >
| > | > so it won't break code like
| > | >
| > | >         if (..)
| > | >                 module_init(x);
| > | >         else
| > | >                 something_else();
| > | >
| > | > (Yes, that code is nonsense but if you're going to nitpick, go all the way...)
| > |
| > | Right. =)
| > | Anyway, besides nitpicking, is there any reason not to include this
| > | code? Or is the added feature considered plain bloat? Yes, the kernel
| > | will become a bit larger, but all the data added by this patch will go
| > | into the init section.
| >
| > Looks like a good idea to me.
| >
| > ---
| > ~Randy
| 
| Can't you disable module-loading with a module? I think so.
| You don't need to modify the kernel. Boot-scripts could
| just load the "final" module and there is nothing that
| can be done to add another module (or even unload existing
| ones).

Sounds likely, but that's not what the patch from Magnus is
even trying to do.  It's purely for boot-time selection of
troublesome modules or devices AFAICT.  I've had a few
occasions to use something like this -- or rebuild a kernel
or remove a device.

---
~Randy

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 7, 2005 7:29 PM, Randy.Dunlap <rddunlap@osdl.org> wrote:
> On Thu, 7 Apr 2005 13:22:57 -0400 (EDT) Richard B. Johnson wrote:
> | Can't you disable module-loading with a module? I think so.
> | You don't need to modify the kernel. Boot-scripts could
> | just load the "final" module and there is nothing that
> | can be done to add another module (or even unload existing
> | ones).
> 
> Sounds likely, but that's not what the patch from Magnus is
> even trying to do.  It's purely for boot-time selection of
> troublesome modules or devices AFAICT.  I've had a few
> occasions to use something like this -- or rebuild a kernel
> or remove a device.

Yes, the idea is to be able to disable misbehaving built-in modules
from the kernel command line. The alternative is of course to disable
the code using the kernel configuration system and rebuild the kernel.

Say a kernel shipped with your favourite distribution crashes your
machine during boot-up - wouldn't it be nice to be able to just
disable the problematic module from the kernel command line instead of
(1) getting the config, (2) getting the distribution-specific patches,
(3) rebuilding the kernel. And if you are lucky you need to (0) setup
a cross compiler and (4) get your hands on a suitable initrd.

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Herbert Xu]

Magnus Damm <magnus.damm@gmail.com> wrote:
> 
> Say a kernel shipped with your favourite distribution crashes your
> machine during boot-up - wouldn't it be nice to be able to just
> disable the problematic module from the kernel command line instead of

Perhaps your favourite distribution could build that as a module to
start with.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 9, 2005 3:42 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Magnus Damm <magnus.damm@gmail.com> wrote:
> >
> > Say a kernel shipped with your favourite distribution crashes your
> > machine during boot-up - wouldn't it be nice to be able to just
> > disable the problematic module from the kernel command line instead of
> 
> Perhaps your favourite distribution could build that as a module to
> start with.

Right. Today distributions can boot from external usb-storage devices,
maybe even from firewire hardware as I am sure you know. I guess they
have support for a device built-in for a reason. I think most
distributions have as streamlined kernels as possible with much code
built as modules - but they would still need some code built-in in the
kernel to have a generic kernel that supports a lot of block devices.
So I think my patch still have a value.

In my case a firewire phy is broken - and, yes - I should fix my
hardware instead of moaning about it here...

Thanks,

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Herbert Xu]

On Sat, Apr 09, 2005 at 11:43:59AM +0200, Magnus Damm wrote:
>
> > Perhaps your favourite distribution could build that as a module to
> > start with.
> 
> Right. Today distributions can boot from external usb-storage devices,
> maybe even from firewire hardware as I am sure you know. I guess they
> have support for a device built-in for a reason. I think most

Perhaps they should start using initramfs then.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 9, 2005 11:48 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Sat, Apr 09, 2005 at 11:43:59AM +0200, Magnus Damm wrote:
> >
> > > Perhaps your favourite distribution could build that as a module to
> > > start with.
> >
> > Right. Today distributions can boot from external usb-storage devices,
> > maybe even from firewire hardware as I am sure you know. I guess they
> > have support for a device built-in for a reason. I think most
> 
> Perhaps they should start using initramfs then.

But how does that help me? I still want to be able to pass a list of
unwanted modules on the kernel command line. Using initramfs and
modules is fine, although I would prefer being able to unload built-in
modules instead - but that is another story. Your suggestion just
pushes the problem to user space. I think the best alternative would
be a combination of kernel-space code (my patch) and awareness of the
command line in the module loader running from initramfs/rootfs.

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Herbert Xu]

On Sat, Apr 09, 2005 at 12:03:45PM +0200, Magnus Damm wrote:
>
> > Perhaps they should start using initramfs then.
> 
> But how does that help me? I still want to be able to pass a list of
> unwanted modules on the kernel command line. Using initramfs and
> modules is fine, although I would prefer being able to unload built-in
> modules instead - but that is another story. Your suggestion just
> pushes the problem to user space. I think the best alternative would

Well you know what they say:

If it can be done in user space, then do it in user space.

Once the drivers are put on the initramfs it is trivial to add code that
disables them based on boot-time options.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 9, 2005 12:07 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Sat, Apr 09, 2005 at 12:03:45PM +0200, Magnus Damm wrote:
> >
> > > Perhaps they should start using initramfs then.
> >
> > But how does that help me? I still want to be able to pass a list of
> > unwanted modules on the kernel command line. Using initramfs and
> > modules is fine, although I would prefer being able to unload built-in
> > modules instead - but that is another story. Your suggestion just
> > pushes the problem to user space. I think the best alternative would
> 
> Well you know what they say:
> 
> If it can be done in user space, then do it in user space.
> 
> Once the drivers are put on the initramfs it is trivial to add code that
> disables them based on boot-time options.

I agree. And if I understand your opinion correctly you believe that
kernels with built-in modules should not have the feature that my
patch provides, right?

I think it would be a nice feature regardless of initramfs or not.

Thanks for your input!

/ magnus

Re: [PATCH][RFC] disable built-in modules V2

[Dave Jones]

On Thu, Apr 07, 2005 at 01:22:57PM -0400, Richard B. Johnson wrote:

 > >| Anyway, besides nitpicking, is there any reason not to include this
 > >| code? Or is the added feature considered plain bloat? Yes, the kernel
 > >| will become a bit larger, but all the data added by this patch will go
 > >| into the init section.
 > >
 > >Looks like a good idea to me.
 > >
 > Can't you disable module-loading with a module? I think so.
 > You don't need to modify the kernel. Boot-scripts could
 > just load the "final" module and there is nothing that
 > can be done to add another module (or even unload existing
 > ones).

Why do you need a module ?

echo 0xFFFEFFFF ?> /proc/sys/kernel/cap-bound

should do this just fine.

		Dave

Re: [PATCH][RFC] disable built-in modules V2

[Horst von Brand]

AsterixTheGaul <asterixthegaul@gmail.com> said:
> > -#define module_init(x) __initcall(x);
> > +#define module_init(x) __initcall(x); __module_init_disable(x);
> 
> It would be better if there is brackets around them... like
> 
> #define module_init(x) { __initcall(x); __module_init_disable(x); }
> 
> then we know it wont break some code like
> 
> if (..)
>  module_init(x);

But happily break:

   if (...)
    module_init(x);
   else
    ...

This should be:

#define module_init(x)  do {__initcall(x); __module_init_disable(x);}while(0)
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: [PATCH][RFC] disable built-in modules V2

[Magnus Damm]

On Apr 7, 2005 4:38 AM, Horst von Brand <vonbrand@inf.utfsm.cl> wrote:
> AsterixTheGaul <asterixthegaul@gmail.com> said:
> > > -#define module_init(x) __initcall(x);
> > > +#define module_init(x) __initcall(x); __module_init_disable(x);
> >
> > It would be better if there is brackets around them... like
> >
> > #define module_init(x) { __initcall(x); __module_init_disable(x); }
> >
> > then we know it wont break some code like
> >
> > if (..)
> >  module_init(x);
> 
> But happily break:
> 
>    if (...)
>     module_init(x);
>    else
>     ...
> 
> This should be:
> 
> #define module_init(x)  do {__initcall(x); __module_init_disable(x);}while(0)

Yes and no. =) Wrapping defines in do {} while(0) is nice when you are
using the defined constants inside functions. module_init() OTOH is
never used inside a function and your suggestion leads to compile
errors:

  CC      arch/i386/kernel/dmi_scan.o
  CC      arch/i386/kernel/bootflag.o
arch/i386/kernel/bootflag.c:99: error: parse error before "do"
arch/i386/kernel/bootflag.c:99: error: parse error before '}' token
make[1]: *** [arch/i386/kernel/bootflag.o] Error 1
make: *** [arch/i386/kernel] Error 2
damm@clementine linux-2.6.12-rc2-disable_builtin $

/ magnus