mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Robert P. J. Day" <rpjday@mindspring.com>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH] Factor out common MODULE_INFO content from module*.h files.
Date: Sun, 20 May 2007 17:12:36 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0705201651570.19270@localhost.localdomain> (raw)
In-Reply-To: <20070520205116.GB25339@uranus.ravnborg.org>

On Sun, 20 May 2007, Sam Ravnborg wrote:

> On Sun, May 20, 2007 at 04:06:40PM -0400, Robert P. J. Day wrote:
> > On Sun, 20 May 2007, Sam Ravnborg wrote:
> >
> > > On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
> > > >
> > > > In order to eventually break the interdependency between the module.h
> > > > and moduleparam.h header files, factor out the common MODULE_INFO
> > > > content into a new header file.
> > >
> > > The moduleinfo.h file looks redundant at first look. Why not push
> > > relevant parts from moduleparam.h (the MODULE_INFO bits) to module.h
> > > and let go of the include of moduleparam.h in module.h (when you
> > > have fixed the users)?
> > >
> > > In this way we do not add an extra .h file. And files that needs
> > > moduleparam.h will anyway always need module.h. But not the other
> > > way around.
> >
> > crap, now i remember why i did it the way i did it.
> >
> > yes, the way you describe it is a simpler solution, but it would break
> > all of the files in the tree that use module parameters and have
> > included *only* module.h, and have been getting away with it all this
> > time only because module.h currently includes moduleparam.h.
> >
> > based on a simple script i have, there are currently 583 files under
> > the drivers/ directory *alone* that are like that.  that is, 583 files
> > that would need to include moduleparam.h instead of module.h simply to
> > continue to compile if the obvious header file fix were made.

> The pain is too high for this. Is seems worthwhile to make the
> change to module.h but adding an additional include to > 500 drivers
> is not worth it.

under the circumstances, is there *any* cleanup worth doing WRT to
this issue?  because of the fact that module.h currently includes
moduleparam.h, developers have been able to get away with being
incredibly sloppy in their includes.

based on a short script i wrote, here are some stats for the drivers/
directory:

$ . ../moduleparam.sh drivers
Number of source files found: 5042
Number of files that include module.h: 3788
Number of files that include moduleparam.h: 358
Number of files that include both: 347
Number of files that include ONLY module.h: 3440
Number of files that include ONLY moduleparam.h: 10
Number of files needing moduleparam.h: 583
Number of files with unnecessary moduleparam.h: 45

interpreting the above:

* there are 347 files that include both module.h and moduleparam.h
when (at the moment) they need to include only the former,

* there are 10 files that include *only* moduleparam.h (which does not
include module.h, so how the heck *those* still build is a mystery),

* there are 583 files that use module parameters in some way that
don't include moduleparam.h, and

* conversely, there are 45 source files that unnecessarily include
moduleparam.h when they don't use module parameters at all.

  at this point, you may be right.  trying to fix this may be more
grief than it's worth.

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

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

  reply	other threads:[~2007-05-20 21:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-20 19:06 Robert P. J. Day
2007-05-20 19:15 ` Sam Ravnborg
2007-05-20 19:51   ` Robert P. J. Day
2007-05-20 20:13     ` Sam Ravnborg
2007-05-20 20:06   ` Robert P. J. Day
2007-05-20 20:51     ` Sam Ravnborg
2007-05-20 21:12       ` Robert P. J. Day [this message]
2007-05-21  4:52         ` Sam Ravnborg
2007-05-21  8:37           ` Robert P. J. Day

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0705201651570.19270@localhost.localdomain \
    --to=rpjday@mindspring.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sam@ravnborg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®