Re: [PATCH] proc_fs.h redux
Em Sun, Oct 28, 2007 at 09:44:41AM +0100, Sam Ravnborg escreveu: As a general rule, I think it better to use includes than use naked forward declarations. Quite the opposite - at least in the kernel source. The general rule is that a .h file shall include the .h files which contain declarations used by said .h files. But naked declarations as above is preferred over including a full header file. yup We see the full header dependency thing to blow off when inline function are used - which is more and more the case. In several cases we have converted inline functions to macros just to simplify the nightmare of header dependencies we have. Arnaldo have a nice script that generate a .ps file showing all the dependencies. He lately posted this URL: http://oops.ghostprotocols.net:81/acme/tcp.h.ps Well, not lately, it has been quite a while. But lets celebrate the fact that there is somebody trying to fight this battle one more time and update this tcp.h dependency tree... /me looks for hviz... and if graphviz is installed, ok: http://www.kernel.org/pub/linux/kernel/people/acme/hviz hviz include/linux/tcp.h 10 | dot -Tpdf /tmp/tcp.h.2007_11.pdf http://oops.ghostprotocols.net:81/acme/tcp.h.2007_11.pdf We still get to sched.h, but before it was: linux/tcp.h - linux/skbuff.h - linux/mm.h - linux/sched.h Nowadays its: linux/tcp.h - linux/sock.h - linux/netdevice.h - linux/interrupt.h - linux/sched.h And I just removed #include linux/sched.h from linux/interrupt.h, because as far as I checked it is completely unnecessary, and the kernel builds just fine :-) - Arnaldo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
On Sat, Oct 27, 2007 at 03:40:04PM -0700, Joe Perches wrote: On Sat, 2007-10-27 at 23:47 +0400, Alexey Dobriyan wrote: Remove proc_fs.h from headers that doesn't really need it. --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -17,6 +17,7 @@ #include linux/errno.h #include linux/sched.h #include linux/kernel.h +#include linux/fs.h Your code doesn't match your patch description. You've got new includes of: linux/fs.h linux/proc_fs.h linux/err.h linux/kref.h and forward declarations of struct proc_dir_entry; struct file_operations; Removal of header A from header B creates compilation breakages for files which were getting A indirectly. So you have to complensate in all such cases. As a general rule, I think it better to use includes than use naked forward declarations. Well, we also want faster compilation and less time wasted when test-compiling patches. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
As a general rule, I think it better to use includes than use naked forward declarations. Quite the opposite - at least in the kernel source. The general rule is that a .h file shall include the .h files which contain declarations used by said .h files. But naked declarations as above is preferred over including a full header file. We see the full header dependency thing to blow off when inline function are used - which is more and more the case. In several cases we have converted inline functions to macros just to simplify the nightmare of header dependencies we have. Arnaldo have a nice script that generate a .ps file showing all the dependencies. He lately posted this URL: http://oops.ghostprotocols.net:81/acme/tcp.h.ps Sam - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
On Sat, Oct 27, 2007 at 03:40:04PM -0700, Joe Perches wrote: and forward declarations of struct proc_dir_entry; struct file_operations; As a general rule, I think it better to use includes than use naked forward declarations. If you go down that route, you end up with _lots_ of circular dependencies - header file X needs Y needs Z which needs X. We've been there, several times. It very quickly becomes quite unmaintainable - you end up with hard to predict behaviour from include files. The only realistic solution is to use forward declarations. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
* Russell King [EMAIL PROTECTED] [2007-10-28 11:34]: If you go down that route, you end up with _lots_ of circular dependencies - header file X needs Y needs Z which needs X. We've been there, several times. It very quickly becomes quite unmaintainable - you end up with hard to predict behaviour from include files. The only realistic solution is to use forward declarations. In header files, yes. But that's not true for implementation files. Thanks, Bernhard - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
On Sun, Oct 28, 2007 at 12:59:52PM +0100, Bernhard Walle wrote: * Russell King [EMAIL PROTECTED] [2007-10-28 11:34]: If you go down that route, you end up with _lots_ of circular dependencies - header file X needs Y needs Z which needs X. We've been there, several times. It very quickly becomes quite unmaintainable - you end up with hard to predict behaviour from include files. The only realistic solution is to use forward declarations. In header files, yes. But that's not true for implementation files. I don't think that needs saying - it's quite obvious. You can't access the contents of structures without their definitions being available. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
* Russell King [EMAIL PROTECTED] [2007-10-28 14:04]: On Sun, Oct 28, 2007 at 12:59:52PM +0100, Bernhard Walle wrote: * Russell King [EMAIL PROTECTED] [2007-10-28 11:34]: If you go down that route, you end up with _lots_ of circular dependencies - header file X needs Y needs Z which needs X. We've been there, several times. It very quickly becomes quite unmaintainable - you end up with hard to predict behaviour from include files. The only realistic solution is to use forward declarations. In header files, yes. But that's not true for implementation files. I don't think that needs saying - it's quite obvious. You can't access the contents of structures without their definitions being available. Of course. But there might be the case where an implementation file doesn't access the structure itself but just passes the pointer to some other function (which is implemented in another file). In that case, you also have the choice between forward declaration and including the header file in the implementation file. Thanks, Bernhard - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
Hi, On Sunday 28 October 2007, Russell King wrote: On Sat, Oct 27, 2007 at 03:40:04PM -0700, Joe Perches wrote: and forward declarations of struct proc_dir_entry; struct file_operations; As a general rule, I think it better to use includes than use naked forward declarations. If you go down that route, you end up with _lots_ of circular dependencies - header file X needs Y needs Z which needs X. We've been there, several times. It very quickly becomes quite unmaintainable - you end up with hard to predict behaviour from include files. The only realistic solution is to use forward declarations. It's unfortunately more complicated than that. What basically needs to be done is to separate declarations from its users (usually inline functions). The problem is to correctly pull the declarations from the asm/... and linux/... header in the correct order. A typical mistake is to put declarations and inline functions in the same linux header and then include some additional from an asm header. For most high level header it's not much of a problem, but let's take linux/list.h as example. struct list_head is used everywhere, but just to get this one definition one also gets quite a few other dependencies as well. bye, Roman - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
On Sat, 2007-10-27 at 23:47 +0400, Alexey Dobriyan wrote: Remove proc_fs.h from headers that doesn't really need it. --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -17,6 +17,7 @@ #include linux/errno.h #include linux/sched.h #include linux/kernel.h +#include linux/fs.h Your code doesn't match your patch description. You've got new includes of: linux/fs.h linux/proc_fs.h linux/err.h linux/kref.h and forward declarations of struct proc_dir_entry; struct file_operations; As a general rule, I think it better to use includes than use naked forward declarations. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
