LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h
@ 2007-11-13 21:32 Julia Lawall
  2007-11-13 21:34 ` Robert P. J. Day
  0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2007-11-13 21:32 UTC (permalink / raw)
  To: khali; +Cc: i2c, linux-kernel, kernel-janitors

Send to:
khali@linux-fr.org,
i2c@lm-sensors.org,
linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org

Subject: [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h


------------------------------------------------------

From: Julia Lawall <julia@diku.dk>

Drop #include <linux/moduleparam.h> in files that also include #include
<linux/module.h>.  module.h includes moduleparam.h already.

The semantic patch implementing this change is as follows:

@ includesmodule @
@@

#include <linux/module.h>

@ depends on includesmodule @
@@

- #include <linux/moduleparam.h>

Signed-off-by: Julia Lawall <julia@diku.dk>
---

diff -u -p -b -B a/drivers/i2c/algos/i2c-algo-pca.c b/drivers/i2c/algos/i2c-algo-pca.c
--- a/drivers/i2c/algos/i2c-algo-pca.c 2007-10-22 11:25:06.000000000 +0200
+++ b/drivers/i2c/algos/i2c-algo-pca.c 2007-11-13 17:49:31.000000000 +0100
@@ -19,7 +19,6 @@

  #include <linux/kernel.h>
  #include <linux/module.h>
-#include <linux/moduleparam.h>
  #include <linux/delay.h>
  #include <linux/slab.h>
  #include <linux/init.h>
diff -u -p -b -B a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
--- a/drivers/i2c/busses/i2c-piix4.c 2007-09-09 08:31:35.000000000 +0200
+++ b/drivers/i2c/busses/i2c-piix4.c 2007-11-13 17:49:33.000000000 +0100
@@ -30,7 +30,6 @@
  */

  #include <linux/module.h>
-#include <linux/moduleparam.h>
  #include <linux/pci.h>
  #include <linux/kernel.h>
  #include <linux/delay.h>
diff -u -p -b -B a/drivers/i2c/busses/i2c-pca-isa.c b/drivers/i2c/busses/i2c-pca-isa.c
--- a/drivers/i2c/busses/i2c-pca-isa.c 2007-06-02 22:32:11.000000000 +0200
+++ b/drivers/i2c/busses/i2c-pca-isa.c 2007-11-13 17:49:34.000000000 +0100
@@ -20,7 +20,6 @@
  #include <linux/kernel.h>
  #include <linux/ioport.h>
  #include <linux/module.h>
-#include <linux/moduleparam.h>
  #include <linux/delay.h>
  #include <linux/slab.h>
  #include <linux/init.h>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h
  2007-11-13 21:32 [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h Julia Lawall
@ 2007-11-13 21:34 ` Robert P. J. Day
  2007-11-14  6:09   ` Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Robert P. J. Day @ 2007-11-13 21:34 UTC (permalink / raw)
  To: Julia Lawall; +Cc: khali, i2c, linux-kernel, kernel-janitors

On Tue, 13 Nov 2007, Julia Lawall wrote:

> Send to:
> khali@linux-fr.org,
> i2c@lm-sensors.org,
> linux-kernel@vger.kernel.org,
> kernel-janitors@vger.kernel.org
>
> Subject: [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h
>
>
> ------------------------------------------------------
>
> From: Julia Lawall <julia@diku.dk>
>
> Drop #include <linux/moduleparam.h> in files that also include #include
> <linux/module.h>.  module.h includes moduleparam.h already.

i'm not convinced that's a good idea.  while module.h does currently
(and unfortunately) include moduleparam.h, there may come a day when
those header files are refactored to actually make sense, at which
point all those missing moduleparam.h inclusions will cause all sorts
of bad things to happen.

rday

p.s.  i tried that cleanup once.  it got ugly in a hurry.

--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h
  2007-11-13 21:34 ` Robert P. J. Day
@ 2007-11-14  6:09   ` Julia Lawall
  2007-11-14  9:21     ` Robert P. J. Day
  0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2007-11-14  6:09 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: khali, i2c, linux-kernel, kernel-janitors

>> From: Julia Lawall <julia@diku.dk>
>>
>> Drop #include <linux/moduleparam.h> in files that also include #include
>> <linux/module.h>.  module.h includes moduleparam.h already.
>
> i'm not convinced that's a good idea.  while module.h does currently
> (and unfortunately) include moduleparam.h, there may come a day when
> those header files are refactored to actually make sense, at which
> point all those missing moduleparam.h inclusions will cause all sorts
> of bad things to happen.

They were removed from the drivers/media files in August:
9c12224a607a4b22ab86784e3394b52810b9507c
Perhaps they should be put back there then?

julia


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h
  2007-11-14  6:09   ` Julia Lawall
@ 2007-11-14  9:21     ` Robert P. J. Day
  2007-11-15 12:19       ` Jean Delvare
  0 siblings, 1 reply; 6+ messages in thread
From: Robert P. J. Day @ 2007-11-14  9:21 UTC (permalink / raw)
  To: Julia Lawall; +Cc: khali, i2c, linux-kernel, kernel-janitors

On Wed, 14 Nov 2007, Julia Lawall wrote:

> > > From: Julia Lawall <julia@diku.dk>
> > >
> > > Drop #include <linux/moduleparam.h> in files that also include #include
> > > <linux/module.h>.  module.h includes moduleparam.h already.
> >
> > i'm not convinced that's a good idea.  while module.h does
> > currently (and unfortunately) include moduleparam.h, there may
> > come a day when those header files are refactored to actually make
> > sense, at which point all those missing moduleparam.h inclusions
> > will cause all sorts of bad things to happen.
>
> They were removed from the drivers/media files in August:
> 9c12224a607a4b22ab86784e3394b52810b9507c
> Perhaps they should be put back there then?

i went down this road before a few months back, checking on how many
source files were unnecessarily including moduleparam.h, and the
result was literally *hundreds*.  i took a shot at cleaning that up,
and even factoring the non-parameter-related content out of that file,
but the change would have been hugely disruptive.

as it is now, since module.h includes moduleparam.h, there is (as you
can see) no reason whatever for *anyone* to include moduleparam.h.
at least right now.

but if anyone ever aspires to tidy up this chaos, then all you'd end
up doing is putting all those includes back in.  so unless there's a
long-term goal here, it would seem safest to just leave things the way
they are.  but that's someone else's decision, not mine.  i'm just
reporting on how quickly things got unpleasant when *i* tried to do
something with this once upon a time.

rday
--

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] drivers/i2c: Drop redundant includes of  moduleparam.h
  2007-11-14  9:21     ` Robert P. J. Day
@ 2007-11-15 12:19       ` Jean Delvare
  2007-11-15 12:42         ` Robert P. J. Day
  0 siblings, 1 reply; 6+ messages in thread
From: Jean Delvare @ 2007-11-15 12:19 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Julia Lawall, i2c, linux-kernel, kernel-janitors

Hi all,

On Wed, 14 Nov 2007 04:21:51 -0500 (EST), Robert P. J. Day wrote:
> On Wed, 14 Nov 2007, Julia Lawall wrote:
> 
> > > > From: Julia Lawall <julia@diku.dk>
> > > >
> > > > Drop #include <linux/moduleparam.h> in files that also include #include
> > > > <linux/module.h>.  module.h includes moduleparam.h already.
> > >
> > > i'm not convinced that's a good idea.  while module.h does
> > > currently (and unfortunately) include moduleparam.h, there may
> > > come a day when those header files are refactored to actually make
> > > sense, at which point all those missing moduleparam.h inclusions
> > > will cause all sorts of bad things to happen.

I share Robert's point of view here. Relying on header files including
each other is usually not a good idea, as build then breaks when the
headers are cleaned up or reorganized.

> > They were removed from the drivers/media files in August:
> > 9c12224a607a4b22ab86784e3394b52810b9507c
> > Perhaps they should be put back there then?
> 
> i went down this road before a few months back, checking on how many
> source files were unnecessarily including moduleparam.h, and the
> result was literally *hundreds*.  i took a shot at cleaning that up,
> and even factoring the non-parameter-related content out of that file,
> but the change would have been hugely disruptive.
> 
> as it is now, since module.h includes moduleparam.h, there is (as you
> can see) no reason whatever for *anyone* to include moduleparam.h.
> at least right now.
> 
> but if anyone ever aspires to tidy up this chaos, then all you'd end
> up doing is putting all those includes back in.  so unless there's a
> long-term goal here, it would seem safest to just leave things the way
> they are.  but that's someone else's decision, not mine.  i'm just
> reporting on how quickly things got unpleasant when *i* tried to do
> something with this once upon a time.

If there's a fundamental reason why module.h has to include
moduleparam.h, then both headers should be merged in a single file. If
not, then both header files should be clearly split and drivers should
include each of them as needed. Either way, header files must be fixed
first, and drivers adjusted in consequence. So, I'm not taking this
patch, sorry.

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] drivers/i2c: Drop redundant includes of  moduleparam.h
  2007-11-15 12:19       ` Jean Delvare
@ 2007-11-15 12:42         ` Robert P. J. Day
  0 siblings, 0 replies; 6+ messages in thread
From: Robert P. J. Day @ 2007-11-15 12:42 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Julia Lawall, i2c, linux-kernel, kernel-janitors

On Thu, 15 Nov 2007, Jean Delvare wrote:

... snip ...

> I share Robert's point of view here. Relying on header files
> including each other is usually not a good idea, as build then
> breaks when the headers are cleaned up or reorganized.

... more snip ...

> If there's a fundamental reason why module.h has to include
> moduleparam.h, then both headers should be merged in a single file.
> If not, then both header files should be clearly split and drivers
> should include each of them as needed. Either way, header files must
> be fixed first, and drivers adjusted in consequence. So, I'm not
> taking this patch, sorry.

keeping this short and sweet, i updated my wiki page on this topic:

http://www.crashcourse.ca/wiki/index.php/Module.h_and_moduleparam.h

i think the fundamental question here is whether this is even
cleanable without massive disruption.

rday

--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-11-15 12:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-11-13 21:32 [PATCH 1/4] drivers/i2c: Drop redundant includes of moduleparam.h Julia Lawall
2007-11-13 21:34 ` Robert P. J. Day
2007-11-14  6:09   ` Julia Lawall
2007-11-14  9:21     ` Robert P. J. Day
2007-11-15 12:19       ` Jean Delvare
2007-11-15 12:42         ` Robert P. J. Day

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).