[U-Boot] [PATCH] flash.h: pull in common.h for types

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 include/flash.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/flash.h b/include/flash.h
index c5e7bf4..886d8ef 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -25,6 +25,9 @@
 #define _FLASH_H_
 
 #ifndef CONFIG_SYS_NO_FLASH
+
+#include <common.h>
+
 /*-----------------------------------------------------------------------
  * FLASH Info: contains chip specific data, per FLASH bank
  */
-- 
1.6.5.2

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1258401513-16014-1-git-send-email-vapier@gentoo.org> you wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  include/flash.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/include/flash.h b/include/flash.h
> index c5e7bf4..886d8ef 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -25,6 +25,9 @@
>  #define _FLASH_H_
>  
>  #ifndef CONFIG_SYS_NO_FLASH
> +
> +#include <common.h>
> +

Why would that be needed? Which problem are you trying to solve?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Always leave room to add an explanation if it doesn't work out.

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Mike Frysinger]

On Monday 16 November 2009 16:31:27 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> >  include/flash.h |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/flash.h b/include/flash.h
> > index c5e7bf4..886d8ef 100644
> > --- a/include/flash.h
> > +++ b/include/flash.h
> > @@ -25,6 +25,9 @@
> >  #define _FLASH_H_
> >
> >  #ifndef CONFIG_SYS_NO_FLASH
> > +
> > +#include <common.h>
> > +
> 
> Why would that be needed? Which problem are you trying to solve?

like the subject very briefly says, 'for types'.  if your source code includes 
flash.h before anything else, it'll fail to compile as flash.h uses types not 
declared implicitly by the compiler.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091116/d6081938/attachment.pgp 

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200911161703.46965.vapier@gentoo.org> you wrote:
>
> > Why would that be needed? Which problem are you trying to solve?
>
> like the subject very briefly says, 'for types'.  if your source code includes 
> flash.h before anything else, it'll fail to compile as flash.h uses types not 
> declared implicitly by the compiler.

I'm not sure if this is needed or even wanted.

We don't attempt to make all header files self-sufficient, or do we?
Does Linux do this?

When looking at man pages for system calls and library funtions it
seems this is not the case; also, I can find wise people argumenting
against self-sufficient headers (and others argumenting in their
favour).


My question: is there a definitive position  somewhere  (for  example
for  the  Linux kernel; I'm sure we don't have one for U-Boot [yet]),
whether system headers should be self-sufficient?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
One does not thank logic.
	-- Sarek, "Journey to Babel", stardate 3842.4

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Scott Wood]

Wolfgang Denk wrote:
> Dear Mike Frysinger,
> 
> In message <200911161703.46965.vapier@gentoo.org> you wrote:
>>> Why would that be needed? Which problem are you trying to solve?
>> like the subject very briefly says, 'for types'.  if your source code includes 
>> flash.h before anything else, it'll fail to compile as flash.h uses types not 
>> declared implicitly by the compiler.
> 
> I'm not sure if this is needed or even wanted.
> 
> We don't attempt to make all header files self-sufficient, or do we?
> Does Linux do this?
> 
> When looking at man pages for system calls and library funtions it
> seems this is not the case; also, I can find wise people argumenting
> against self-sufficient headers (and others argumenting in their
> favour).
> 
> 
> My question: is there a definitive position  somewhere  (for  example
> for  the  Linux kernel; I'm sure we don't have one for U-Boot [yet]),
> whether system headers should be self-sufficient?

I'd say they should be self-sufficient, in that the inclusion of the 
header itself should not fail if I haven't included some arbitrary other 
header.  I don't see what the argument would be for not doing this.

I don't know whether Linux has a specific policy on this, but I haven't 
noticed many problems in this regard, and when I did find one in the 
kernel a few years back I didn't get any argument when I submitted a 
patch to fix it.

Which man pages are you looking at?

-Scott

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Wolfgang Denk]

Dear Scott Wood,

In message <4B031158.20501@freescale.com> you wrote:
>
> > My question: is there a definitive position  somewhere  (for  example
> > for  the  Linux kernel; I'm sure we don't have one for U-Boot [yet]),
> > whether system headers should be self-sufficient?
> 
> I'd say they should be self-sufficient, in that the inclusion of the 
> header itself should not fail if I haven't included some arbitrary other 
> header.  I don't see what the argument would be for not doing this.

Well, Theo de Raadt says for example "... people would be able to
include less files; indeed, almost be careless about what they
include. But this would not increase portability in any way. And
'make build' would probably, if it was taken the nth degree, take
twice as long. Therefore there is no benefit for the crazy rule you
suggest..." - see
http://www.mail-archive.com/tech at openbsd.org/msg00425.html

> I don't know whether Linux has a specific policy on this, but I haven't 
> noticed many problems in this regard, and when I did find one in the 
> kernel a few years back I didn't get any argument when I submitted a 
> patch to fix it.
> 
> Which man pages are you looking at?

Well, for example:

open(2):
	SYNOPSIS
	       #include <sys/types.h>
	       #include <sys/stat.h>
	       #include <fcntl.h>

mknod(2):
	SYNOPSIS
	       #include <sys/types.h>
	       #include <sys/stat.h>
	       #include <fcntl.h>
	       #include <unistd.h>

stat(2):
	SYNOPSIS
	       #include <sys/types.h>
	       #include <sys/stat.h>
	       #include <unistd.h>

Why do we need these lists of #includes? WHy doe - for example -
<sys/stat.h> not auto-include anything it might need? 

To me this seems to be an indication that there is no intention to
make headers self-sufficent, but I am absolutely not sure.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To know how another being, another creature feels -  that  is  impos-
sible.                  - Terry Pratchett, _The Dark Side of the Sun_

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Scott Wood]

Wolfgang Denk wrote:
> Dear Scott Wood,
> 
> In message <4B031158.20501@freescale.com> you wrote:
>>> My question: is there a definitive position  somewhere  (for  example
>>> for  the  Linux kernel; I'm sure we don't have one for U-Boot [yet]),
>>> whether system headers should be self-sufficient?
>> I'd say they should be self-sufficient, in that the inclusion of the 
>> header itself should not fail if I haven't included some arbitrary other 
>> header.  I don't see what the argument would be for not doing this.
> 
> Well, Theo de Raadt says for example "... people would be able to
> include less files; indeed, almost be careless about what they
> include. But this would not increase portability in any way. And
> 'make build' would probably, if it was taken the nth degree, take
> twice as long. Therefore there is no benefit for the crazy rule you
> suggest..." - see
> http://www.mail-archive.com/tech at openbsd.org/msg00425.html

Making the headers self-sufficient is not an excuse for the including C 
file to not include everything it uses -- it just makes the C file not 
have to care about other stuff in the header that it does not intend to 
use, or which is a header implementation detail.

>> Which man pages are you looking at?
> 
> Well, for example:
> 
> open(2):
> 	SYNOPSIS
> 	       #include <sys/types.h>
> 	       #include <sys/stat.h>
> 	       #include <fcntl.h>
> 
> mknod(2):
> 	SYNOPSIS
> 	       #include <sys/types.h>
> 	       #include <sys/stat.h>
> 	       #include <fcntl.h>
> 	       #include <unistd.h>
> 
> stat(2):
> 	SYNOPSIS
> 	       #include <sys/types.h>
> 	       #include <sys/stat.h>
> 	       #include <unistd.h>
> 
> Why do we need these lists of #includes? WHy doe - for example -
> <sys/stat.h> not auto-include anything it might need? 
> 
> To me this seems to be an indication that there is no intention to
> make headers self-sufficent, but I am absolutely not sure.

That is just listing the headers to include in order to have access to 
all of the facilities described.  However, any one of those headers 
should be able to be included by itself without the inclusion alone 
causing the build to fail.  A quick test under Linux/glibc shows this to 
be the case (i.e. no failures).

-Scott

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Mike Frysinger]

On Tuesday 17 November 2009 16:56:58 Wolfgang Denk wrote:
> Scott Wood wrote:
> > > My question: is there a definitive position  somewhere  (for  example
> > > for  the  Linux kernel; I'm sure we don't have one for U-Boot [yet]),
> > > whether system headers should be self-sufficient?
> >
> > I'd say they should be self-sufficient, in that the inclusion of the
> > header itself should not fail if I haven't included some arbitrary other
> > header.  I don't see what the argument would be for not doing this.
> 
> Well, Theo de Raadt says for example "... people would be able to
> include less files; indeed, almost be careless about what they
> include. But this would not increase portability in any way. And
> 'make build' would probably, if it was taken the nth degree, take
> twice as long. Therefore there is no benefit for the crazy rule you
> suggest..." - see
> http://www.mail-archive.com/tech at openbsd.org/msg00425.html

i disagree with this using, ironically, the same base logic, but a different 
conclusion:
http://sourceware.org/ml/libc-alpha/2006-08/msg00064.html

also, i think a self contained system like u-boot which has full control at 
the api level can be better at this than a user interface which really sits on 
top of an abi and has to deal with a lot of crap from user code.

while i'm not asking for you or anyone else to audit header paths here as i 
think that level of enforcement will bog things down, small patches from 
people who choose to fix things should be merged.

> > I don't know whether Linux has a specific policy on this, but I haven't
> > noticed many problems in this regard, and when I did find one in the
> > kernel a few years back I didn't get any argument when I submitted a
> > patch to fix it.

ive semi-frequently post fixes to linux headers so that you can include just 
that header and have it work.  i have yet to hear anyone complain; rather 
every one has been merged (ignoring issues unrelated to the original purpose).

> > Which man pages are you looking at?
> 
> Well, for example:
> 
> open(2):
> 	SYNOPSIS
> 	       #include <sys/types.h>
> 	       #include <sys/stat.h>
> 	       #include <fcntl.h>
> 
> mknod(2):
> 	SYNOPSIS
> 	       #include <sys/types.h>
> 	       #include <sys/stat.h>
> 	       #include <fcntl.h>
> 	       #include <unistd.h>
> 
> stat(2):
> 	SYNOPSIS
> 	       #include <sys/types.h>
> 	       #include <sys/stat.h>
> 	       #include <unistd.h>
> 
> Why do we need these lists of #includes? WHy doe - for example -
> <sys/stat.h> not auto-include anything it might need?
> 
> To me this seems to be an indication that there is no intention to
> make headers self-sufficent, but I am absolutely not sure.

i'm pretty sure your man page example is an unrelated issue.  the include list 
does not imply that any one of those headers cannot be included by itself 
first.  if you read the full text of the man page, there are many 
defines/options which may be utilized.  if you want to use different pieces, 
you might have to include the related header.  so the full header list is 
given to cover any one piece of code in the man page by itself.

going by my point, none of these will result in a build failure:
for h in sys/types.h sys/stat.h unistd.h fcntl.h ; do
	gcc -include $h -x c -c - -o /dev/null < /dev/null
done

going by the man page point of open(2), this code will compile:
#include <fcntl.h>
main() { open("foo", O_RDONLY); }
but if you want to create a file with symbolic perms, you need sys/stat.h:
#include <fcntl.h>
#include <sys/stat.h>
main() { open("foo", O_CREAT, S_IRWXG); }
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091117/330b1a78/attachment.pgp 

[U-Boot] [PATCH] flash.h: pull in common.h for types

[J. William Campbell]

Mike Frysinger wrote:
> On Tuesday 17 November 2009 16:56:58 Wolfgang Denk wrote:
>   
>> Scott Wood wrote:
>>     
>>>> My question: is there a definitive position  somewhere  (for  example
>>>> for  the  Linux kernel; I'm sure we don't have one for U-Boot [yet]),
>>>> whether system headers should be self-sufficient?
>>>>         
>>> I'd say they should be self-sufficient, in that the inclusion of the
>>> header itself should not fail if I haven't included some arbitrary other
>>> header.  I don't see what the argument would be for not doing this.
>>>       
>> Well, Theo de Raadt says for example "... people would be able to
>> include less files; indeed, almost be careless about what they
>> include. But this would not increase portability in any way. And
>> 'make build' would probably, if it was taken the nth degree, take
>> twice as long. Therefore there is no benefit for the crazy rule you
>> suggest..." - see
>> http://www.mail-archive.com/tech at openbsd.org/msg00425.html
>>     
>
> i disagree with this using, ironically, the same base logic, but a different 
> conclusion:
> http://sourceware.org/ml/libc-alpha/2006-08/msg00064.html
>
> also, i think a self contained system like u-boot which has full control at 
> the api level can be better at this than a user interface which really sits on 
> top of an abi and has to deal with a lot of crap from user code.
>
> while i'm not asking for you or anyone else to audit header paths here as i 
> think that level of enforcement will bog things down, small patches from 
> people who choose to fix things should be merged.
>
>   
FWIW, I think one needs to be very careful with this reasoning. It is 
clear that experienced and capable programmers disagree on the correct 
approach to this problem. It is also true that the logical structure of 
the include chain is important. A "crap" interface is going to be hard 
to maintain no matter how you do it. The problem is, "crap" has a way of 
sneaking into well-designed interfaces in the form of small patches to 
"fix" things. (I am not saying this is one of them). I have observed 
that in any large program, as time goes on, more and more things get 
included in more and more places. Hiding this fact by doing the 
inclusion in other header files often obscures the drift towards a very 
polluted name space, where editing just about any include file requires 
the entire system to be rebuilt. Some would say, so what, it is easy to 
do. However, these situations often result in hard to find bugs. The 
question of "which modules can access this variable" becomes "all of 
them" because of (badly designed?)  interfaces being included in other 
interfaces without the user being explicitly aware of this. Often I see 
in patch critiques the statement "This belongs in a header file", which 
may be true, but can lead to a bunch of un-related things being stuck 
together in a header file just to meet the requirement. Those bad 
choices then get included in other header files and we are off to the 
races. Since the users are not explicitly aware they are pulling in a 
bunch of stuff that has nothing to do with the main purpose of the 
module being written, there is often no incentive to clean up bad header 
designs until things get so bad it is almost impossible to untangle 
things. So when adding an include to an include file, think hard whether 
you are bringing along a lot of things that are "not required" and that 
one would not expect to be exposed in all files using the modified 
include file. If you are bringing along a lot of "baggage", perhaps the 
interface you are including could use some re-work.

Best Regards,
Bill Campbell
>>> I don't know whether Linux has a specific policy on this, but I haven't
>>> noticed many problems in this regard, and when I did find one in the
>>> kernel a few years back I didn't get any argument when I submitted a
>>> patch to fix it.
>>>       
>
> ive semi-frequently post fixes to linux headers so that you can include just 
> that header and have it work.  i have yet to hear anyone complain; rather 
> every one has been merged (ignoring issues unrelated to the original purpose).
>
>   
>>> Which man pages are you looking at?
>>>       
>> Well, for example:
>>
>> open(2):
>> 	SYNOPSIS
>> 	       #include <sys/types.h>
>> 	       #include <sys/stat.h>
>> 	       #include <fcntl.h>
>>
>> mknod(2):
>> 	SYNOPSIS
>> 	       #include <sys/types.h>
>> 	       #include <sys/stat.h>
>> 	       #include <fcntl.h>
>> 	       #include <unistd.h>
>>
>> stat(2):
>> 	SYNOPSIS
>> 	       #include <sys/types.h>
>> 	       #include <sys/stat.h>
>> 	       #include <unistd.h>
>>
>> Why do we need these lists of #includes? WHy doe - for example -
>> <sys/stat.h> not auto-include anything it might need?
>>
>> To me this seems to be an indication that there is no intention to
>> make headers self-sufficent, but I am absolutely not sure.
>>     
>
> i'm pretty sure your man page example is an unrelated issue.  the include list 
> does not imply that any one of those headers cannot be included by itself 
> first.  if you read the full text of the man page, there are many 
> defines/options which may be utilized.  if you want to use different pieces, 
> you might have to include the related header.  so the full header list is 
> given to cover any one piece of code in the man page by itself.
>
> going by my point, none of these will result in a build failure:
> for h in sys/types.h sys/stat.h unistd.h fcntl.h ; do
> 	gcc -include $h -x c -c - -o /dev/null < /dev/null
> done
>
> going by the man page point of open(2), this code will compile:
> #include <fcntl.h>
> main() { open("foo", O_RDONLY); }
> but if you want to create a file with symbolic perms, you need sys/stat.h:
> #include <fcntl.h>
> #include <sys/stat.h>
> main() { open("foo", O_CREAT, S_IRWXG); }
> -mike
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>   

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200911171918.39290.vapier@gentoo.org> you wrote:
>
> > > I don't know whether Linux has a specific policy on this, but I haven't
> > > noticed many problems in this regard, and when I did find one in the
> > > kernel a few years back I didn't get any argument when I submitted a
> > > patch to fix it.
>
> ive semi-frequently post fixes to linux headers so that you can include just 
> that header and have it work.  i have yet to hear anyone complain; rather 
> every one has been merged (ignoring issues unrelated to the original purpose).

Note that I don't complain. Your patch just made me aware that I don;t
know if there is any official poicy for this anywhere, and my initial
search was not really successfull.

Are you aware of any such official declarations for Linux, or glibc,
or similar?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
EMACS belongs in <sys/errno.h>: Editor too big!

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Mike Frysinger]

On Wednesday 18 November 2009 17:28:46 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > > > I don't know whether Linux has a specific policy on this, but I
> > > > haven't noticed many problems in this regard, and when I did find one
> > > > in the kernel a few years back I didn't get any argument when I
> > > > submitted a patch to fix it.
> >
> > ive semi-frequently post fixes to linux headers so that you can include
> > just that header and have it work.  i have yet to hear anyone complain;
> > rather every one has been merged (ignoring issues unrelated to the
> > original purpose).
> 
> Note that I don't complain. Your patch just made me aware that I don;t
> know if there is any official poicy for this anywhere, and my initial
> search was not really successfull.
> 
> Are you aware of any such official declarations for Linux, or glibc,
> or similar?

in terms of public headers that random end users include, i'm not aware of any 
standard and/or official declaration for either project, or any large 
important open source project for that matter
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091118/9077c360/attachment.pgp 

[U-Boot] [PATCH] flash.h: pull in common.h for types

[Mike Frysinger]

On Monday 16 November 2009 14:58:33 Mike Frysinger wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  include/flash.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/include/flash.h b/include/flash.h
> index c5e7bf4..886d8ef 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -25,6 +25,9 @@
>  #define _FLASH_H_
> 
>  #ifndef CONFIG_SYS_NO_FLASH
> +
> +#include <common.h>
> +
>  /*-----------------------------------------------------------------------
>   * FLASH Info: contains chip specific data, per FLASH bank
>   */

so should i drop this ?  i noticed this issue because of a bare-bones board 
port i did, but i can just as easily add the common.h include to my one or two 
files if this isnt going to be merged ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100117/1389fa90/attachment.pgp