Patchwork [U-Boot,v3,01/16] Implement autoconf header file

login
register
mail settings
Submitter Simon Glass
Date Feb. 26, 2013, 4:10 p.m.
Message ID <1361895069-7343-2-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/223278/
State Deferred
Delegated to: Tom Rini
Headers show

Comments

Simon Glass - Feb. 26, 2013, 4:10 p.m.
Add support for generating an autoconf.h header file that can be used in
the source instead of #ifdef.

For example, instead of:

 #ifdef CONFIG_VERSION_VARIABLE
	setenv("ver", version_string);  /* set version variable */
 #endif

you can do:

	if (autoconf_version_variable())
		setenv("ver", version_string);  /* set version variable */

The compiler will ensure that the dead code is eliminated, so the result
is the same.

Where the value of the CONFIG define is 0, you can use the autoconf_has...()
form. For example CONFIG_BOOTDELAY can be -ve, 0 or +ve, but if it is
defined at all, it affects behaviour:

 #if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0)
	s = getenv ("bootdelay");
 #endif

So we use:

	if (autoconf_has_bootdelay() && autoconf_bootdelay() >= 0)
		s = getenv ("bootdelay");

This later form should only be used for such 'difficult' defines where a
zero value still means that the CONFIG should be considered to be defined.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Update config_xxx() to autoconf_xxx() in comments/README/sed
- Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed
- Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]*
- Rename sed scripts to more useful names

Changes in v2:
- Split out changes to main.c into separate patches
- Fix up a few errors and comments in the original RFC
- Use autoconf_...() instead of config_...()
- Use autoconf_has_...() instead of config_..._enabled()
- Add a grep to the sed/sort pipe to speed up processing

 Makefile                       | 43 ++++++++++++++++++++-
 README                         | 87 ++++++++++++++++++++++++++++++++++++++++--
 include/common.h               |  3 ++
 include/config_drop.h          | 17 +++++++++
 tools/scripts/define2value.sed | 37 ++++++++++++++++++
 tools/scripts/define2zero.sed  | 32 ++++++++++++++++
 6 files changed, 215 insertions(+), 4 deletions(-)
 create mode 100644 include/config_drop.h
 create mode 100644 tools/scripts/define2value.sed
 create mode 100644 tools/scripts/define2zero.sed

Patch

diff --git a/Makefile b/Makefile
index fc18dd4..8ba068d 100644
--- a/Makefile
+++ b/Makefile
@@ -614,6 +614,7 @@  updater:
 # parallel sub-makes creating .depend files simultaneously.
 depend dep:	$(TIMESTAMP_FILE) $(VERSION_FILE) \
 		$(obj)include/autoconf.mk \
+		$(obj)include/generated/autoconf.h \
 		$(obj)include/generated/generic-asm-offsets.h \
 		$(obj)include/generated/asm-offsets.h
 		for dir in $(SUBDIRS) $(CPUDIR) $(LDSCRIPT_MAKEFILE_DIR) ; do \
@@ -688,6 +689,45 @@  $(obj)include/autoconf.mk: $(obj)include/config.h
 		sed -n -f tools/scripts/define2mk.sed > $@.tmp && \
 	mv $@.tmp $@
 
+# Create a C header file where every '#define CONFIG_XXX value' becomes
+# '#define autoconf_xxx() value', or '#define autoconf_xxx() 0' where the
+# CONFIG is not used by this board configuration. This allows C code to do
+# things like 'if (autoconf_xxx())' and have the compiler remove the dead code,
+# instead of using '#ifdef CONFIG_XXX...#endif'. Note that in most cases
+# if the autoconf_...() returns 0 then the option is not enabled. In some rare
+# cases such as CONFIG_BOOTDELAY, the config can be enabled but still have a
+# a value of 0. So in addition we a #define autoconf_has_xxx(), setting the
+# value to 0 if the option is disabled, 1 if enabled. This last feature will
+# hopefully be deprecated soon.
+# The file is regenerated when any U-Boot header file changes.
+$(obj)include/generated/autoconf.h: $(obj)include/config.h
+	@$(XECHO) Generating $@ ; \
+	set -e ; \
+	: Extract the config macros to a C header file ; \
+	$(CPP) $(CFLAGS) -DDO_DEPS_ONLY -dM include/common.h | \
+		sed -n -f tools/scripts/define2value.sed > $@.tmp; \
+	: Regenerate our list of all config macros if neeed ; \
+	if [ ! -f $@-all.tmp ] || \
+		find $(src) -name '*.h' -type f -newer $@-all.tmp | \
+			egrep -qv 'include/(autoconf.h|generated|config.h)'; \
+			then \
+		: Extract all config macros from all C header files ; \
+		: We can grep for CONFIG since the value will be dropped ; \
+		( \
+			find ${src} -name "*.h" -type f | xargs \
+			cat | grep CONFIG | \
+			sed -n -f tools/scripts/define2zero.sed \
+		) | sort | uniq > $@-all.tmp; \
+	fi; \
+	: Extract the enabled config macros to a C header file ; \
+	$(CPP) $(CFLAGS) -DDO_DEPS_ONLY -dM include/common.h | \
+		sed -n -f tools/scripts/define2zero.sed | \
+			sort > $@-enabled.tmp; \
+	set -e ; \
+	: Find CONFIGs that are not enabled ; \
+	comm -13 $@-enabled.tmp $@-all.tmp >>$@.tmp && \
+	mv $@.tmp $@
+
 $(obj)include/generated/generic-asm-offsets.h:	$(obj)include/autoconf.mk.dep \
 	$(obj)lib/asm-offsets.s
 	@$(XECHO) Generating $@
@@ -770,7 +810,8 @@  include/license.h: tools/bin2header COPYING
 unconfig:
 	@rm -f $(obj)include/config.h $(obj)include/config.mk \
 		$(obj)board/*/config.tmp $(obj)board/*/*/config.tmp \
-		$(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep
+		$(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep \
+		$(obj)include/generated/autoconf.h
 
 %_config::	unconfig
 	@$(MKCONFIG) -A $(@:_config=)
diff --git a/README b/README
index d8cb394..e6f3c04 100644
--- a/README
+++ b/README
@@ -5434,11 +5434,92 @@  Notes:
 * If you modify existing code, make sure that your new code does not
   add to the memory footprint of the code ;-) Small is beautiful!
   When adding new features, these should compile conditionally only
-  (using #ifdef), and the resulting code with the new feature
-  disabled must not need more memory than the old code without your
-  modification.
+  (avoiding #ifdef where at all possible), and the resulting code with
+  the new feature disabled must not need more memory than the old code
+  without your modification.
 
 * Remember that there is a size limit of 100 kB per message on the
   u-boot mailing list. Bigger patches will be moderated. If they are
   reasonable and not too big, they will be acknowledged. But patches
   bigger than the size limit should be avoided.
+
+
+Use of #ifdef:
+--------------
+Many parts of the U-Boot code base are sprinkled with #ifdefs. This makes
+different boards compile different versions of the source code, meaning
+that we must build all boards to check for failures. It is easy to misspell
+an #ifdef and there is not as much checking of this by the compiler. For
+someone coming new into the code base, #ifdefs are a big turn-off. Multiple
+dependent #ifdefs are harder to do than with if..then..else. Variable
+declarations must be #idefed as well as the code that uses them, often much
+later in the file/function. #ifdef indents don't match code indents and
+have their own separate indent feature. Overall, excessive use of #idef
+hurts readability and makes the code harder to modify and refactor.
+
+In an effort to reduce the use of #ifdef in U-Boot, without requiring lots
+of special static inlines all over the header files, a single autoconf.h
+header file with lower-case function-type macros has been made available.
+
+This file has either:
+
+#	#define autoconf_xxx() value
+
+for enabled options, or:
+
+#	#define autoconf_xxx() 0
+
+for disabled options. You can therefore generally change code like this:
+
+	#ifdef CONFIG_XXX
+		do_something
+	#else
+		do_something_else
+	#endif
+
+to this:
+
+	if (autoconf_xxx())
+		do_something;
+	else
+		do_something_else;
+
+The compiler will see that autoconf_xxx() evalutes to a constant and will
+eliminate the dead code. The resulting code (and code size) is the same.
+
+Multiple #ifdefs can be converted also:
+
+	#if defined(CONFIG_XXX) && !defined(CONFIG_YYY)
+		do_something
+	#endif
+
+	if (autoconf_xxx() && !autoconf_yyy())
+		do_something;
+
+Where the macro evaluates to a string, it will be non-NULL, so the above
+will work whether the macro is a string or a number.
+
+This takes care of almost all CONFIG macros. Unfortunately there are a few
+cases where a value of 0 does not mean the option is disabled. For example
+CONFIG_BOOTDELAY can be defined to 0, which means that the bootdelay
+code should be used, but with a value of 0. To get around this and other
+sticky cases, an addition macro with an 'has_' prefix is provided, where
+the value is always either 0 or 1:
+
+	// Will work even if boaard config has '#define CONFIG_BOOTDELAY 0'
+	if (autoconf_has_bootdelay())
+		do_something;
+
+(Probably such config options should be deprecated and then we can remove
+this feature)
+
+U-Boot already has a Makefile scheme to permit files to be easily included
+based on CONFIG. This can be used where the code to be compiled exists in
+its own source file. So the following rules apply:
+
+  1. Use #ifdef to conditionally compile an exported function or variable
+  2. Use ordinary C code with autoconf_xxx() everywhere else
+  3. Mark your functions and data structures static where possible
+  4. Use the autoconf_has_xxx() variants only if essential
+  5. When changing existing code, first create a new patch to replace
+        #ifdefs in the surrounding area
diff --git a/include/common.h b/include/common.h
index 4ad17ea..491783b 100644
--- a/include/common.h
+++ b/include/common.h
@@ -35,6 +35,9 @@  typedef volatile unsigned short vu_short;
 typedef volatile unsigned char	vu_char;
 
 #include <config.h>
+#ifndef DO_DEPS_ONLY
+#include <generated/autoconf.h>
+#endif
 #include <asm-offsets.h>
 #include <linux/bitops.h>
 #include <linux/types.h>
diff --git a/include/config_drop.h b/include/config_drop.h
new file mode 100644
index 0000000..bf68b50
--- /dev/null
+++ b/include/config_drop.h
@@ -0,0 +1,17 @@ 
+/*
+ * Copyright 2013 Google, Inc
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License Version 2. This file is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _CONFIG_DROP_H
+#define _CONFIG_DROP_H
+
+/* Options which don't seem to be defined by any header in U-Boot */
+#define CONFIG_MENUPROMPT	"Auto-boot prompt"
+#define CONFIG_MENUKEY
+#define CONFIG_UPDATE_TFTP
+
+#endif
diff --git a/tools/scripts/define2value.sed b/tools/scripts/define2value.sed
new file mode 100644
index 0000000..205f9fe
--- /dev/null
+++ b/tools/scripts/define2value.sed
@@ -0,0 +1,37 @@ 
+#
+# Sed script to parse CPP macros and generate a list of CONFIG macros
+#
+# This converts:
+#	#define CONFIG_XXX value
+#into:
+#	#define autoconf_xxx() value
+#	#define autoconf_has_xxx() 1
+
+# Macros with parameters are ignored.
+# (Note we avoid + since it doesn't appear to work)
+/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ {
+	d
+}
+
+# Only process values prefixed with #define CONFIG_
+/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*/ {
+	# Strip the #define prefix
+	s/#define[ \t]*CONFIG_/autoconf_/;
+	# Change to form CONFIG_*=VALUE
+	s/[\t ][\t ]*/=/;
+	# Handle lines with no value
+	s/^\([^=]*\)$/\1=/;
+	# Drop trailing spaces
+	s/ *$//;
+	# Change empty values to '1'
+	s/=$/=1/;
+	# Add #define at the start
+	s/^\([^=]*\)=/#define \L\1() /
+	# print the line
+	p
+	# Create autoconf_has_...(), value 1
+	s/().*/() 1/
+	s/\(autoconf_\)/\1has_/
+	# print the line
+	p
+}
diff --git a/tools/scripts/define2zero.sed b/tools/scripts/define2zero.sed
new file mode 100644
index 0000000..95e6860
--- /dev/null
+++ b/tools/scripts/define2zero.sed
@@ -0,0 +1,32 @@ 
+#
+# Sed script to parse CPP macros and generate a list of autoconf macros
+#
+# This converts:
+#	#define CONFIG_XXX value
+#into:
+#	#define autoconf_xxx() 0
+#	#define autoconf_has_xxx() 0
+
+# Macros with parameters are ignored.
+# (Note we avoid + since it doesn't appear to work)
+/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ {
+	s/.*//
+}
+
+# Only process values prefixed with #define CONFIG_
+/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*/ {
+	# Strip the #define prefix
+	s/#define *//;
+	# Remove the value
+	s/[ \t].*//;
+	# Convert to lower case, prepend #define
+	s/CONFIG_\(.*\)/#define autoconf_\L\1/
+	# Append 0
+	s/$/() 0/
+	# print the line
+	p
+	# Create autoconf_has_...(), value 0
+	s/\(autoconf_\)/\1has_/
+	# print the line
+	p
+}