[U-Boot] [PATCH] flash.h: pull in common.h for types
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 */
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
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
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
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
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@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
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@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
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@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
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@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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
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
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
participants (4)
-
J. William Campbell -
Mike Frysinger -
Scott Wood -
Wolfgang Denk