[PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

The header file <linux/tty_driver.h> should include <asm/termbits.h>
to pick up the definition of "struct ktermios"; otherwise, you run the
risk of seeing:

include/linux/tty_driver.h:142: warning: 'struct ktermios' \
	declared inside parameter list
include/linux/tty_driver.h:142: warning: its scope is only \
	this definition or declaration, which is probably not what you want

Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>

---

  i'm *guessing* this is the solution to the above issue.


diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 85c95cd..b687174 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -123,6 +123,7 @@
 #include <linux/fs.h>
 #include <linux/list.h>
 #include <linux/cdev.h>
+#include <asm/termbits.h>

 struct tty_struct;


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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Jan Engelhardt]

On Jun 8 2007 16:30, Alan Cox wrote:
>
>> 
>> The header file <linux/tty_driver.h> should include <asm/termbits.h>
>> to pick up the definition of "struct ktermios"; otherwise, you run the
>> risk of seeing:
>
>Risk - which compile combinations does it actually show up with and is it
>not better to fix the C file in question not grow the horribly tangled
>include deps further

struct ktermios; /* that's it */


	Jan
-- 

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

On Fri, 8 Jun 2007, Alan Cox wrote:

> On Fri, 8 Jun 2007 09:29:12 -0400 (EDT)
> "Robert P. J. Day" <rpjday@mindspring.com> wrote:
>
> > The header file <linux/tty_driver.h> should include
> > <asm/termbits.h> to pick up the definition of "struct ktermios";
> > otherwise, you run the risk of seeing:
>
> Risk - which compile combinations does it actually show up with and
> is it not better to fix the C file in question not grow the horribly
> tangled include deps further

i don't see how fixing the C file solves this issue.  the header file
<linux/tty_driver.h> clearly refers to "struct ktermios", for which it
won't have a definition unless it includes <asm/termbits.h>.

you can hack a solution by explicitly including the latter first, of
course, but i don't see how that constitutes a proper fix.  or am i
misreading something here?

rday

p.s.  this isn't just theoretical -- this bit me this morning as i was
writing a simple module to flash the keyboard LEDs, just for the
entertainment value.

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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Alan Cox]

On Fri, 8 Jun 2007 09:29:12 -0400 (EDT)
"Robert P. J. Day" <rpjday@mindspring.com> wrote:

> 
> The header file <linux/tty_driver.h> should include <asm/termbits.h>
> to pick up the definition of "struct ktermios"; otherwise, you run the
> risk of seeing:

Risk - which compile combinations does it actually show up with and is it
not better to fix the C file in question not grow the horribly tangled
include deps further

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

On Fri, 8 Jun 2007, Jan Engelhardt wrote:

>
> On Jun 8 2007 16:30, Alan Cox wrote:
> >
> >>
> >> The header file <linux/tty_driver.h> should include <asm/termbits.h>
> >> to pick up the definition of "struct ktermios"; otherwise, you run the
> >> risk of seeing:
> >
> >Risk - which compile combinations does it actually show up with and
> >is it not better to fix the C file in question not grow the
> >horribly tangled include deps further
>
> struct ktermios; /* that's it */

um ... what does that mean?  consider this snippet from
<linux/tty_driver.h>:

struct tty_driver {
	...
        struct ktermios init_termios; /* Initial termios */
	...

  doesn't this suggest that you better have a definition of that
ktermios struct before this point?

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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

On Fri, 8 Jun 2007, Alan Cox wrote:

> > > >Risk - which compile combinations does it actually show up with and
> > > >is it not better to fix the C file in question not grow the
> > > >horribly tangled include deps further
> > >
> > > struct ktermios; /* that's it */
> >
> > um ... what does that mean?  consider this snippet from
> > <linux/tty_driver.h>:
>
> Which compile combination shows the sequence you are reporting ?

sorry, i don't understand the question.  i'm just running "make" on a
simple module i wrote, using the prototypical in-directory makefile to
build out-of-tree.

the first four lines of my module source file are:

#include <linux/module.h>
#include <linux/init.h>
#include <asm/termbits.h>               /* Shouldn't be necessary */
#include <linux/tty_driver.h>           /* For struct tty_driver */

  if i omit the inclusion of the termbits.h header, i get the
aforementioned error.  beyond that, i'm unclear on what you're asking.

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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Alan Cox]

> > >Risk - which compile combinations does it actually show up with and
> > >is it not better to fix the C file in question not grow the
> > >horribly tangled include deps further
> >
> > struct ktermios; /* that's it */
> 
> um ... what does that mean?  consider this snippet from
> <linux/tty_driver.h>:

Which compile combination shows the sequence you are reporting ?

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

On Fri, 8 Jun 2007, Alan Cox wrote:

> > i don't see how fixing the C file solves this issue.  the header
> > file <linux/tty_driver.h> clearly refers to "struct ktermios", for
> > which it won't have a definition unless it includes
> > <asm/termbits.h>.
>
> Anyone using tty stuff will include <linux/tty.h> to get all the
> stuff needed an that includes asm/termios.h which includes
> asm/termbits.h

that may well be, but i don't think counting on the choice or order of
header file inclusions represents a really robust approach to software
design.

if a header file needs a definition/declaration that exists in another
header file, it should include that other header file.  any other
approach is wrong.

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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Alan Cox]

> i don't see how fixing the C file solves this issue.  the header file
> <linux/tty_driver.h> clearly refers to "struct ktermios", for which it
> won't have a definition unless it includes <asm/termbits.h>.

Anyone using tty stuff will include <linux/tty.h> to get all the stuff
needed an that includes asm/termios.h which includes asm/termbits.h

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Alan Cox]

> if a header file needs a definition/declaration that exists in another
> header file, it should include that other header file.  any other
> approach is wrong.

And if you try this in the real world for a project as large as the
kernel you will die in a mass of unresolvable dependancy loops. The
current behaviour is quite intentional

Alan

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

On Fri, 8 Jun 2007, Alan Cox wrote:

> > if a header file needs a definition/declaration that exists in another
> > header file, it should include that other header file.  any other
> > approach is wrong.
>
> And if you try this in the real world for a project as large as the
> kernel you will die in a mass of unresolvable dependancy loops. The
> current behaviour is quite intentional

what utter nonsense.  if i include tty.h, then i will be including
header files like workqueue.h and mutex.h and perhaps other rubbish i
have absolutely no interest in and don't want to waste my time
parsing.

if i choose to invest an extra minute or two and be more selective and
include only those headers which represent content i actually *need*,
then i have every right to be a bit put out if my build suddenly
fails.

if you don't agree with that patch, fine.  but suggesting that it's
going to result in "a mass of unresolvable dependancy loops" is sheer
laziness.

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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Alan Cox]

> if you don't agree with that patch, fine.  but suggesting that it's
> going to result in "a mass of unresolvable dependancy loops" is sheer
> laziness.

No its "sheer experience", frequently painful. The tty headers and the
scheduler headers are particularly problematic for getting tangled.

Alan

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Robert P. J. Day]

On Fri, 8 Jun 2007, Alan Cox wrote:

> > if you don't agree with that patch, fine.  but suggesting that it's
> > going to result in "a mass of unresolvable dependancy loops" is sheer
> > laziness.
>
> No its "sheer experience", frequently painful. The tty headers and
> the scheduler headers are particularly problematic for getting
> tangled.

ok, i'll defer to your experience.  but can we at least apply my patch
to protect console_struct.h from multiple inclusion?  that one really
*is* annoying.

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

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

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Alan Cox]

> ok, i'll defer to your experience.  but can we at least apply my patch
> to protect console_struct.h from multiple inclusion?  that one really
> *is* annoying.

I've not seen that one but send it - multiple inclusion is a different
problem altogether

Re: [PATCH] Include <asm/termbits.h> from <linux/tty_driver.h>.

[Jan Engelhardt]

On Jun 8 2007 11:42, Robert P. J. Day wrote:
>> >
>> > um ... what does that mean?  consider this snippet from
>> > <linux/tty_driver.h>:

That's right. Each .h file should be "compilable" on their own.

>>
>> Which compile combination shows the sequence you are reporting ?
>
>sorry, i don't understand the question.  i'm just running "make" on a
>simple module i wrote, using the prototypical in-directory makefile to
>build out-of-tree.
>
>the first four lines of my module source file are:
>
>#include <linux/module.h>
>#include <linux/init.h>
>#include <asm/termbits.h>               /* Shouldn't be necessary */
>#include <linux/tty_driver.h>           /* For struct tty_driver */
>
>  if i omit the inclusion of the termbits.h header, i get the
>aforementioned error.  beyond that, i'm unclear on what you're asking.
>
>rday
>-- 
>========================================================================
>Robert P. J. Day
>Linux Consulting, Training and Annoying Kernel Pedantry
>Waterloo, Ontario, CANADA
>
>http://fsdev.net/wiki/index.php?title=Main_Page
>========================================================================
>

	Jan
--