Skip to contentSkip to conversation

headerdep: a tool for detecting inclusion cycles in header files

PATCH In review Vegard Nossum 2 participants 4 messages May 3, 2008 – Dec 18, 2008 Kbuild

lore
Share & tools
lore
#0 lore raw

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
index 39516bf..eeab061 100644
@@ -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
#1 lore raw
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.h
include/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

#2 lore raw
Quoted 11 lines · Sam Ravnborg

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.h
include/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,
from linux/topology.h
from linux/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 inclusion
Quoted 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 24d6e19682ce2b7ffa6c032fb0199dbcabcc1a5b Mon 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
index 9a49960..d4b13d9 100644
@@ -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
#3 lore raw
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