headerdep: a tool for detecting inclusion cycles in header files
PATCH 1/1 headerdep: a tool for detecting inclusion cycles in header files
Hi,
Here comes v2 of this tool. I've addressed most of your nitpicks and specific comments. Running 'make headerdep', will go through all headers (instead of source files as v1 did). Running with the --graph option will produce a dependency graph in the 'dot' language (GraphViz).
Note that I am not really making an attempt to include this in the kernel itself; I am submitting it in the hope that somebody will find it useful.
For Sam: Writing this in sparse is a waste of time. Sparse is good for parsing C code; this script only cares about a subset of the C _preprocessor_ parsing. Perl is good at this, and the script was finished in a couple of hours. Sparse is C and would require a bit more work, I'm sure.
The most useful tool, as suggested by several people (Matthew Wilcox, Sam Ravnborg), would be the one that detects unused headers and missing (implicit) includes. That is indeed a job for sparse. But not a job for me, or at least not yet :-)
So again, I have no expectations for this to go in the kernel -- but in my opinion a tool that could be used on the kernel headers with some advantage.
Happy hacking! :-)
Vegard
From: Vegard Nossum <[email protected]>
Subject: [PATCH] headerdep: add a tool to detect inclusion cycles in header files
--- Makefile | 5 ++ scripts/headerdep.pl | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 0 deletions(-) create mode 100755 scripts/headerdep.pl
Makefile +5 -0
@@ -1021,6 +1021,10 @@ PHONY += headers_check headers_check: headers_install $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.headersinst ARCH=$(SRCARCH) obj=include HDRCHECK=1 +PHONY += headerdep +headerdep: + @find include/ -name '*.h' | xargs --max-args 1 scripts/headerdep.pl + # --------------------------------------------------------------------------- # Modules
@@ -1210,6 +1214,7 @@ help: @if [ -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \ echo ' headers_check - Sanity check on exported headers'; \ fi + @echo ' headerdep - Detect inclusion cycles in headers' @echo '' @echo 'Kernel packaging:' @$(MAKE) $(build)=$(package-dir) help
scripts/headerdep.pl new file +177 -0
177 lines not on this page · raw message
Signature · 1 line
1.5.4.1
Quoted 15 lines · Vegard Nossum
Hi,
Here comes v2 of this tool. I've addressed most of your nitpicks and specific comments. Running 'make headerdep', will go through all headers (instead of source files as v1 did). Running with the --graph option will produce a dependency graph in the 'dot' language (GraphViz).
Note that I am not really making an attempt to include this in the kernel itself; I am submitting it in the hope that somebody will find it useful.
For Sam: Writing this in sparse is a waste of time. Sparse is good for parsing C code; this script only cares about a subset of the C _preprocessor_ parsing. Perl is good at this, and the script was finished in a couple of hours. Sparse is C and would require a bit more work, I'm sure.
Quoted 7 lines
The most useful tool, as suggested by several people (Matthew Wilcox, Sam Ravnborg), would be the one that detects unused headers and missing (implicit) includes. That is indeed a job for sparse. But not a job for me, or at least not yet :-)
So again, I have no expectations for this to go in the kernel -- but in my opinion a tool that could be used on the kernel headers with some advantage.
Hi Vegard.
Digging through my mailbox I found this patch of yours. I tried to apply it and saw following output:
In file included from linux/mmzone.h,
from linux/topology.h
from linux/mmzone.h
from linux/gfp.h
from linux/slub_def.h
from linux/slab.h
from linux/percpu.h
from asm-generic/irq_regs.hinclude/asm-xtensa/irq_regs.h: warning: recursive header inclusion
But looking at the file (include/asm-xtensa/irq_regs.h) did not help me understand where to look to fix it.
Can you pinpoint the culprint better so this is becoming useful?
I would also like to see this integrated with checkincludes.pl as both script do some simple static checks of the header files.
Additional bonus if we can reuse this to do better check of our exported headers.
Sam
Quoted 11 lines · Sam Ravnborg
I tried to apply it and saw following output:
In file included fromlinux/mmzone.h,
fromlinux/topology.h
fromlinux/mmzone.h
fromlinux/gfp.h
fromlinux/slub_def.h
fromlinux/slab.h
fromlinux/percpu.h
fromasm-generic/irq_regs.hinclude/asm-xtensa/irq_regs.h: warning: recursive header inclusion
Quoted 2 lines
But looking at the file (
include/asm-xtensa/irq_regs.h) did not help me understand where to look to fix it.
The program was run with include/asm-xtensa/irq_regs.h as input. So it displays the whole chain of inclusion. The cycle is just the upper part:
Quoted 3 lines
In file included from
linux/mmzone.h,
fromlinux/topology.h
fromlinux/mmzone.h
...so that's where to look in general. I checked mmzone.h and topology.h, and they do indeed include each other.
Quoted 2 lines
Can you pinpoint the culprint better so this is becoming useful?
Line numbers would be nice, I agree. We can also have just the cycle stand out a bit more. This is the new format:
$ perl scripts/headerdep.pl include/asm-xtensa/irq_regs.h
In file included from linux/mmzone.h,
from linux/topology.h:32
from linux/mmzone.h:763 <-- here
from linux/gfp.h:4
from linux/slub_def.h:10
from linux/slab.h:152
from linux/percpu.h:5
from asm-generic/irq_regs.h:15
include/asm-xtensa/irq_regs.h:1: warning: recursive header inclusionQuoted 5 lines
I would also like to see this integrated with checkincludes.pl as both script do some simple static checks of the header files.
Additional bonus if we can reuse this to do better check of our exported headers.
This one is probably a bit too noisy? But if we can eliminate the existing cycles, it would be nice, I agree. Thanks for the feedback! :)
Vegard
Quoted 1 line
From
24d6e19682ce2b7ffa6c032fb0199dbcabcc1a5bMon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Tue, 16 Dec 2008 12:33:43 +0100
Subject: [PATCH] headerdep: add a tool to detect inclusion cycles in header files #3
--- Makefile | 7 ++- scripts/headerdep.pl | 193 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 1 deletions(-) create mode 100755 scripts/headerdep.pl
Makefile +6 -1
@@ -1022,6 +1022,10 @@ include/linux/version.h: $(srctree)/Makefile FORCE include/linux/utsrelease.h: include/config/kernel.release FORCE $(call filechk,utsrelease.h) +PHONY += headerdep +headerdep: + @find include/ -name '*.h' | xargs --max-args 1 scripts/headerdep.pl + # --------------------------------------------------------------------------- PHONY += depend dep
@@ -1270,7 +1274,8 @@ help: @echo ' versioncheck - Sanity check on version.h usage' @echo ' includecheck - Check for duplicate included header files' @echo ' export_report - List the usages of all exported symbols' - @echo ' headers_check - Sanity check on exported headers'; \ + @echo ' headers_check - Sanity check on exported headers' + @echo ' headerdep - Detect inclusion cycles in headers'; \ echo '' @echo 'Kernel packaging:' @$(MAKE) $(build)=$(package-dir) help
scripts/headerdep.pl new file +193 -0
193 lines not on this page · raw message
Signature · 1 line
1.5.6.5
Quoted 3 lines
This one is probably a bit too noisy? But if we can eliminate the existing cycles, it would be nice, I agree. Thanks for the feedback! :)
Applied to kbuild-next.
Sam