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 v4: - Rebase on current master

Changes in v3: - Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]* - Rename sed scripts to more useful names - Update config_xxx() to autoconf_xxx() in comments/README/sed - Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed

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

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

diff --git a/Makefile b/Makefile index 2d18d27..3cc6db0 100644 --- a/Makefile +++ b/Makefile @@ -636,6 +636,7 @@ depend dep: $(TIMESTAMP_FILE) $(VERSION_FILE) \ $(obj)include/spl-autoconf.mk \ $(obj)include/tpl-autoconf.mk \ $(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 \ @@ -734,6 +735,45 @@ $(obj)include/spl-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)include/spl-autoconf.mk \ $(obj)include/tpl-autoconf.mk \ @@ -833,7 +873,8 @@ unconfig: $(obj)board/*/config.tmp $(obj)board/*/*/config.tmp \ $(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep \ $(obj)include/spl-autoconf.mk \ - $(obj)include/tpl-autoconf.mk + $(obj)include/tpl-autoconf.mk \ + $(obj)include/generated/autoconf.h

%_config:: unconfig @$(MKCONFIG) -A $(@:_config=) diff --git a/README b/README index f0eedbb..428332c 100644 --- a/README +++ b/README @@ -5912,11 +5912,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 409515f..f07f740 100644 --- a/include/common.h +++ b/include/common.h @@ -19,6 +19,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 +}

Change this feature to use autoconf instead of #ifdef.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v4: - Rebase on current master - Tidy up code style nits with new checkpatch

Changes in v3: - Fix missing && in if() statement - Remove unneeded retry_min variable

Changes in v2: None

common/main.c | 73 ++++++++++++++++++++++++++--------------------------------- 1 file changed, 32 insertions(+), 41 deletions(-)

diff --git a/common/main.c b/common/main.c index 6f475f0..54c5df8 100644 --- a/common/main.c +++ b/common/main.c @@ -49,17 +49,11 @@ static char * delete_char (char *buffer, char *p, int *colp, int *np, int plen); static const char erase_seq[] = "\b \b"; /* erase sequence */ static const char tab_seq[] = " "; /* used to expand TABs */

-#ifdef CONFIG_BOOT_RETRY_TIME static uint64_t endtime = 0; /* must be set, default is instant timeout */ static int retry_time = -1; /* -1 so can call readline before main_loop */ -#endif

#define endtick(seconds) (get_ticks() + (uint64_t)(seconds) * get_tbclk())

-#ifndef CONFIG_BOOT_RETRY_MIN -#define CONFIG_BOOT_RETRY_MIN CONFIG_BOOT_RETRY_TIME -#endif - #ifdef CONFIG_MODEM_SUPPORT int do_mdm_init = 0; extern void mdm_init(void); /* defined in board.c */ @@ -158,11 +152,10 @@ static int abortboot_keyed(int bootdelay) delaykey[i].retry ? "delay" : "stop");

-# ifdef CONFIG_BOOT_RETRY_TIME - /* don't retry auto boot */ - if (! delaykey[i].retry) + /* don't retry auto boot? */ + if (autoconf_boot_retry_time() && + !delaykey[i].retry) retry_time = -1; -# endif abort = 1; } } @@ -352,9 +345,8 @@ static void process_boot_delay(void) #if defined(CONFIG_MENU_SHOW) bootdelay = menu_show(bootdelay); #endif -# ifdef CONFIG_BOOT_RETRY_TIME - init_cmd_timeout (); -# endif /* CONFIG_BOOT_RETRY_TIME */ + if (autoconf_boot_retry_time()) + init_cmd_timeout();

#ifdef CONFIG_POST if (gd->flags & GD_FLG_POSTFAIL) { @@ -483,14 +475,12 @@ void main_loop(void) for (;;); #else for (;;) { -#ifdef CONFIG_BOOT_RETRY_TIME - if (rc >= 0) { + if (autoconf_boot_retry_time() && rc >= 0) { /* Saw enough of a valid command to * restart the timeout. */ reset_cmd_timeout(); } -#endif len = readline (CONFIG_SYS_PROMPT);

flag = 0; /* assume no special flags for now */ @@ -498,19 +488,16 @@ void main_loop(void) strcpy (lastcommand, console_buffer); else if (len == 0) flag |= CMD_FLAG_REPEAT; -#ifdef CONFIG_BOOT_RETRY_TIME - else if (len == -2) { + else if (autoconf_boot_retry_time() && len == -2) { /* -2 means timed out, retry autoboot */ - puts ("\nTimed out waiting for command\n"); -# ifdef CONFIG_RESET_TO_RETRY + puts("\nTimed out waiting for command\n"); /* Reinit board to run initialization code again */ - do_reset (NULL, 0, 0, NULL); -# else - return; /* retry autoboot */ -# endif + if (autoconf_reset_to_retry()) + do_reset(NULL, 0, 0, NULL); + else + return; /* retry autoboot */ } -#endif

if (len == -1) puts ("<INTERRUPT>\n"); @@ -525,6 +512,10 @@ void main_loop(void) #endif /*CONFIG_SYS_HUSH_PARSER*/ }

+/* + * Use ifdef here for the benefit of those archs not using + * -ffunction-sections, since these functions are exported. + */ #ifdef CONFIG_BOOT_RETRY_TIME /*************************************************************************** * initialize command line timeout @@ -536,10 +527,10 @@ void init_cmd_timeout(void) if (s != NULL) retry_time = (int)simple_strtol(s, NULL, 10); else - retry_time = CONFIG_BOOT_RETRY_TIME; + retry_time = autoconf_boot_retry_time();

- if (retry_time >= 0 && retry_time < CONFIG_BOOT_RETRY_MIN) - retry_time = CONFIG_BOOT_RETRY_MIN; + if (retry_time >= 0 && retry_time < autoconf_boot_retry_min()) + retry_time = autoconf_boot_retry_min(); }

/*************************************************************************** @@ -761,13 +752,13 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len, cread_add_str(buf, init_len, 1, &num, &eol_num, buf, *len);

while (1) { -#ifdef CONFIG_BOOT_RETRY_TIME - while (!tstc()) { /* while no incoming data */ - if (retry_time >= 0 && get_ticks() > endtime) - return (-2); /* timed out */ - WATCHDOG_RESET(); + if (autoconf_boot_retry_time()) { + while (!tstc()) { /* while no incoming data */ + if (retry_time >= 0 && get_ticks() > endtime) + return -2; /* timed out */ + WATCHDOG_RESET(); + } } -#endif if (first && timeout) { uint64_t etime = endtick(timeout);

@@ -1039,14 +1030,14 @@ int readline_into_buffer(const char *const prompt, char *buffer, int timeout) col = plen;

for (;;) { -#ifdef CONFIG_BOOT_RETRY_TIME - while (!tstc()) { /* while no incoming data */ - if (retry_time >= 0 && get_ticks() > endtime) - return (-2); /* timed out */ - WATCHDOG_RESET(); + if (autoconf_boot_retry_time()) { + while (!tstc()) { /* while no incoming data */ + if (retry_time >= 0 && get_ticks() > endtime) + return -2; /* timed out */ + WATCHDOG_RESET(); + } } -#endif - WATCHDOG_RESET(); /* Trigger watchdog, if needed */ + WATCHDOG_RESET(); /* Trigger watchdog, if needed */

#ifdef CONFIG_SHOW_ACTIVITY while (!tstc()) {

Use autoconf to make process_boot_delay() be compiled always, and adjust the caller and related functions as needed.

Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v4: - Split out new patch to remove #ifdefs around process_boot_delay()

Changes in v3: None Changes in v2: None

common/main.c | 71 ++++++++++++++++++++++++++--------------------------------- 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/common/main.c b/common/main.c index 5768a16..824f5f7 100644 --- a/common/main.c +++ b/common/main.c @@ -63,8 +63,6 @@ extern void mdm_init(void); /* defined in board.c */ * Watch for 'delay' seconds for autoboot stop or autoboot delay string. * returns: 0 - no key string, allow autoboot 1 - got key string, abort */ -#if defined(CONFIG_BOOTDELAY) -# if defined(CONFIG_AUTOBOOT_KEYED) static int abortboot_keyed(int bootdelay) { int abort = 0; @@ -157,8 +155,6 @@ static int abortboot_keyed(int bootdelay) return abort; }

-# else /* !defined(CONFIG_AUTOBOOT_KEYED) */ - static int menukey;

static int abortboot_normal(int bootdelay) @@ -212,17 +208,14 @@ static int abortboot_normal(int bootdelay)

return abort; } -# endif /* CONFIG_AUTOBOOT_KEYED */

static int abortboot(int bootdelay) { -#ifdef CONFIG_AUTOBOOT_KEYED - return abortboot_keyed(bootdelay); -#else - return abortboot_normal(bootdelay); -#endif + if (autoconf_autoboot_keyed()) + return abortboot_keyed(bootdelay); + else + return abortboot_normal(bootdelay); } -#endif /* CONFIG_BOOTDELAY */

/* * Runs the given boot command securely. Specifically: @@ -238,7 +231,6 @@ static int abortboot(int bootdelay) * printing the error message to console. */

-#if defined(CONFIG_BOOTDELAY) && defined(CONFIG_OF_CONTROL) static void secure_boot_cmd(char *cmd) { cmd_tbl_t *cmdtp; @@ -279,22 +271,21 @@ static void process_fdt_options(const void *blob)

/* Add an env variable to point to a kernel payload, if available */ addr = fdtdec_get_config_int(gd->fdt_blob, "kernel-offset", 0); - if (addr) - setenv_addr("kernaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr)); + if (addr) { + setenv_addr("kernaddr", + (void *)(autoconf_sys_text_base() + addr)); + }

/* Add an env variable to point to a root disk, if available */ addr = fdtdec_get_config_int(gd->fdt_blob, "rootdisk-offset", 0); - if (addr) - setenv_addr("rootaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr)); + if (addr) { + setenv_addr("rootaddr", + (void *)(autoconf_sys_text_base() + addr)); + } } -#endif /* CONFIG_OF_CONTROL */

-#ifdef CONFIG_BOOTDELAY static void process_boot_delay(void) { -#ifdef CONFIG_OF_CONTROL - char *env; -#endif char *s; int bootdelay; #ifdef CONFIG_BOOTCOUNT_LIMIT @@ -311,7 +302,7 @@ static void process_boot_delay(void) #endif /* CONFIG_BOOTCOUNT_LIMIT */

s = getenv ("bootdelay"); - bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY; + bootdelay = s ? (int)simple_strtol(s, NULL, 10) : autoconf_bootdelay();

#ifdef CONFIG_OF_CONTROL bootdelay = fdtdec_get_config_int(gd->fdt_blob, "bootdelay", @@ -341,23 +332,24 @@ static void process_boot_delay(void) else #endif /* CONFIG_BOOTCOUNT_LIMIT */ s = getenv ("bootcmd"); -#ifdef CONFIG_OF_CONTROL - /* Allow the fdt to override the boot command */ - env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd"); - if (env) - s = env; + if (autoconf_of_control()) { + char *env;

- process_fdt_options(gd->fdt_blob); + /* Allow the fdt to override the boot command */ + env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd"); + if (env) + s = env;

- /* - * If the bootsecure option was chosen, use secure_boot_cmd(). - * Always use 'env' in this case, since bootsecure requres that the - * bootcmd was specified in the FDT too. - */ - if (fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0)) - secure_boot_cmd(env); + process_fdt_options(gd->fdt_blob);

-#endif /* CONFIG_OF_CONTROL */ + /* + * If the bootsecure option was chosen, use secure_boot_cmd(). + * Always use 'env' in this case, since bootsecure requres that + * the bootcmd was specified in the FDT too. + */ + if (fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0)) + secure_boot_cmd(env); + }

debug ("### main_loop: bootcmd="%s"\n", s ? s : "<UNDEFINED>");

@@ -381,7 +373,6 @@ static void process_boot_delay(void) } #endif /* CONFIG_MENUKEY */ } -#endif /* CONFIG_BOOTDELAY */

void main_loop(void) { @@ -441,9 +432,9 @@ void main_loop(void) update_tftp(0UL); #endif /* CONFIG_UPDATE_TFTP */

-#ifdef CONFIG_BOOTDELAY - process_boot_delay(); -#endif + if (autoconf_has_bootdelay() && autoconf_bootdelay() >= 0) + process_boot_delay(); + /* * Main Loop for Monitor Command Processing */

Allow parser selection to make use of autoconf instead of #ifdefs.

Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v4: - Rebase on current master

Changes in v3: None Changes in v2: None

common/main.c | 87 +++++++++++++++++++++++++++------------------------------- include/hush.h | 2 -- 2 files changed, 41 insertions(+), 48 deletions(-)

diff --git a/common/main.c b/common/main.c index b744234..060da11 100644 --- a/common/main.c +++ b/common/main.c @@ -366,12 +366,10 @@ static void process_boot_delay(void)

void main_loop(void) { -#ifndef CONFIG_SYS_HUSH_PARSER static char lastcommand[CONFIG_SYS_CBSIZE] = { 0, }; int len; int rc = 1; int flag; -#endif #ifdef CONFIG_PREBOOT char *p; #endif @@ -428,12 +426,11 @@ void main_loop(void) /* * Main Loop for Monitor Command Processing */ -#ifdef CONFIG_SYS_HUSH_PARSER - parse_file_outer(); - /* This point is never reached */ - for (;;); -#else - for (;;) { + if (autoconf_sys_hush_parser()) { + parse_file_outer(); + /* This point is never reached */ + for (;;); + } else { if (autoconf_boot_retry_time() && rc >= 0) { /* Saw enough of a valid command to * restart the timeout. @@ -468,7 +465,6 @@ void main_loop(void) lastcommand[0] = 0; } } -#endif /*CONFIG_SYS_HUSH_PARSER*/ }

/* @@ -1158,7 +1154,6 @@ int parse_line (char *line, char *argv[])

/****************************************************************************/

-#ifndef CONFIG_SYS_HUSH_PARSER static void process_macros (const char *input, char *output) { char c, prev; @@ -1366,7 +1361,6 @@ static int builtin_run_command(const char *cmd, int flag)

return rc ? rc : repeatable; } -#endif

/* * Run a command using the selected parser. @@ -1377,22 +1371,21 @@ static int builtin_run_command(const char *cmd, int flag) */ int run_command(const char *cmd, int flag) { -#ifndef CONFIG_SYS_HUSH_PARSER - /* - * builtin_run_command can return 0 or 1 for success, so clean up - * its result. - */ - if (builtin_run_command(cmd, flag) == -1) - return 1; - - return 0; -#else - return parse_string_outer(cmd, + if (autoconf_sys_hush_parser()) { + return parse_string_outer(cmd, FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP); -#endif + } else { + /* + * builtin_run_command can return 0 or 1 for success, so + * clean up its result. + */ + if (builtin_run_command(cmd, flag) == -1) + return 1; + + return 0; + } }

-#ifndef CONFIG_SYS_HUSH_PARSER /** * Execute a list of command separated by ; or \n using the built-in parser. * @@ -1433,7 +1426,6 @@ static int builtin_run_command_list(char *cmd, int flag)

return rcode; } -#endif

int run_command_list(const char *cmd, int len, int flag) { @@ -1443,13 +1435,16 @@ int run_command_list(const char *cmd, int len, int flag)

if (len == -1) { len = strlen(cmd); -#ifdef CONFIG_SYS_HUSH_PARSER - /* hush will never change our string */ - need_buff = 0; -#else - /* the built-in parser will change our string if it sees \n */ - need_buff = strchr(cmd, '\n') != NULL; -#endif + if (autoconf_sys_hush_parser()) { + /* hush will never change our string */ + need_buff = 0; + } else { + /* + * the built-in parser will change our string if it + * sees \n + */ + need_buff = strchr(cmd, '\n') != NULL; + } } if (need_buff) { buff = malloc(len + 1); @@ -1458,20 +1453,20 @@ int run_command_list(const char *cmd, int len, int flag) memcpy(buff, cmd, len); buff[len] = '\0'; } -#ifdef CONFIG_SYS_HUSH_PARSER - rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON); -#else - /* - * This function will overwrite any \n it sees with a \0, which - * is why it can't work with a const char *. Here we are making - * using of internal knowledge of this function, to avoid always - * doing a malloc() which is actually required only in a case that - * is pretty rare. - */ - rcode = builtin_run_command_list(buff, flag); - if (need_buff) - free(buff); -#endif + if (autoconf_sys_hush_parser()) { + rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON); + } else { + /* + * This function will overwrite any \n it sees with a \0, which + * is why it can't work with a const char *. Here we are making + * using of internal knowledge of this function, to avoid always + * doing a malloc() which is actually required only in a case + * that is pretty rare. + */ + rcode = builtin_run_command_list(buff, flag); + if (need_buff) + free(buff); + }

return rcode; } diff --git a/include/hush.h b/include/hush.h index 595303a..652de69 100644 --- a/include/hush.h +++ b/include/hush.h @@ -20,7 +20,5 @@ int set_local_var(const char *s, int flg_export); void unset_local_var(const char *name); char *get_local_var(const char *s);

-#if defined(CONFIG_HUSH_INIT_VAR) extern int hush_init_var (void); #endif -#endif

Remove #ifdefs in favour of autoconf for this code. This involves removing a few unnecessary #ifdefs in headers also.

We have two versions of the code - one that handles command line editing and one that is just a simple implementation. Create a new function called readline_into_buffer() which calls either cread_line() or the new simple_readline(), created to hold the 'simple' code.

The cread_print_hist_list() function is not actually used anywhere, so punt it.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v4: None Changes in v3: None Changes in v2: None

common/main.c | 164 ++++++++++++++++++++++++------------------------------ include/command.h | 2 - include/common.h | 2 - 3 files changed, 73 insertions(+), 95 deletions(-)

diff --git a/common/main.c b/common/main.c index 060da11..94f1e24 100644 --- a/common/main.c +++ b/common/main.c @@ -497,8 +497,6 @@ void reset_cmd_timeout(void) } #endif

-#ifdef CONFIG_CMDLINE_EDITING - /* * cmdline-editing related codes from vivi. * Author: Janghoon Lyu nandy@mizi.com @@ -601,27 +599,6 @@ static char* hist_next(void) return (ret); }

-#ifndef CONFIG_CMDLINE_EDITING -static void cread_print_hist_list(void) -{ - int i; - unsigned long n; - - n = hist_num - hist_max; - - i = hist_add_idx + 1; - while (1) { - if (i > hist_max) - i = 0; - if (i == hist_add_idx) - break; - printf("%s\n", hist_list[i]); - n++; - i++; - } -} -#endif /* CONFIG_CMDLINE_EDITING */ - #define BEGINNING_OF_LINE() { \ while (num) { \ getcmd_putch(CTL_BACKSPACE); \ @@ -883,27 +860,27 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len, REFRESH_TO_EOL(); continue; } -#ifdef CONFIG_AUTO_COMPLETE - case '\t': { - int num2, col; + case '\t': + if (autoconf_auto_complete()) { + int num2, col;

- /* do not autocomplete when in the middle */ - if (num < eol_num) { - getcmd_cbeep(); - break; - } + /* do not autocomplete when in the middle */ + if (num < eol_num) { + getcmd_cbeep(); + break; + }

- buf[num] = '\0'; - col = strlen(prompt) + eol_num; - num2 = num; - if (cmd_auto_complete(prompt, buf, &num2, &col)) { - col = num2 - num; - num += col; - eol_num += col; + buf[num] = '\0'; + col = strlen(prompt) + eol_num; + num2 = num; + if (cmd_auto_complete(prompt, buf, &num2, + &col)) { + col = num2 - num; + num += col; + eol_num += col; + } + break; } - break; - } -#endif default: cread_add_char(ichar, insert, &num, &eol_num, buf, *len); break; @@ -919,8 +896,6 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len, return 0; }

-#endif /* CONFIG_CMDLINE_EDITING */ - /****************************************************************************/

/* @@ -942,46 +917,14 @@ int readline (const char *const prompt) return readline_into_buffer(prompt, console_buffer, 0); }

- -int readline_into_buffer(const char *const prompt, char *buffer, int timeout) +static int simple_readline(const char *const prompt, int plen, char *p, + int timeout) { - char *p = buffer; -#ifdef CONFIG_CMDLINE_EDITING - unsigned int len = CONFIG_SYS_CBSIZE; - int rc; - static int initted = 0; - - /* - * History uses a global array which is not - * writable until after relocation to RAM. - * Revert to non-history version if still - * running from flash. - */ - if (gd->flags & GD_FLG_RELOC) { - if (!initted) { - hist_init(); - initted = 1; - } - - if (prompt) - puts (prompt); - - rc = cread_line(prompt, p, &len, timeout); - return rc < 0 ? rc : len; - - } else { -#endif /* CONFIG_CMDLINE_EDITING */ char * p_buf = p; int n = 0; /* buffer index */ - int plen = 0; /* prompt length */ int col; /* output column cnt */ char c;

- /* print prompt */ - if (prompt) { - plen = strlen (prompt); - puts (prompt); - } col = plen;

for (;;) { @@ -994,12 +937,12 @@ int readline_into_buffer(const char *const prompt, char *buffer, int timeout) } WATCHDOG_RESET(); /* Trigger watchdog, if needed */

-#ifdef CONFIG_SHOW_ACTIVITY - while (!tstc()) { - show_activity(0); - WATCHDOG_RESET(); + if (autoconf_show_activity()) { + while (!tstc()) { + show_activity(0); + WATCHDOG_RESET(); + } } -#endif c = getc();

/* @@ -1046,14 +989,20 @@ int readline_into_buffer(const char *const prompt, char *buffer, int timeout) */ if (n < CONFIG_SYS_CBSIZE-2) { if (c == '\t') { /* expand TABs */ -#ifdef CONFIG_AUTO_COMPLETE - /* if auto completion triggered just continue */ - *p = '\0'; - if (cmd_auto_complete(prompt, console_buffer, &n, &col)) { - p = p_buf + n; /* reset */ - continue; + if (autoconf_auto_complete()) { + /* + * if auto completion triggered + * just continue + */ + *p = '\0'; + if (cmd_auto_complete(prompt, + console_buffer, + &n, &col)) { + /* reset */ + p = p_buf + n; + continue; + } } -#endif puts (tab_seq+(col&07)); col += 8 - (col&07); } else { @@ -1075,9 +1024,42 @@ int readline_into_buffer(const char *const prompt, char *buffer, int timeout) } } } -#ifdef CONFIG_CMDLINE_EDITING +} + +int readline_into_buffer(const char *const prompt, char *buffer, int timeout) +{ + unsigned int len = CONFIG_SYS_CBSIZE; + int rc; + static int initted; + + /* + * History uses a global array which is not + * writable until after relocation to RAM. + * Revert to non-history version if still + * running from flash. + */ + if (autoconf_cmdline_editing() && (gd->flags & GD_FLG_RELOC)) { + if (!initted) { + hist_init(); + initted = 1; + } + + if (prompt) + puts(prompt); + + rc = cread_line(prompt, buffer, &len, timeout); + return rc < 0 ? rc : len; + + } else { + int plen = 0; /* prompt length */ + + /* print prompt */ + if (prompt) { + plen = strlen(prompt); + puts(prompt); + } + return simple_readline(prompt, plen, buffer, timeout); } -#endif }

/****************************************************************************/ diff --git a/include/command.h b/include/command.h index f782779..d3ed6ed 100644 --- a/include/command.h +++ b/include/command.h @@ -59,10 +59,8 @@ cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len);

extern int cmd_usage(const cmd_tbl_t *cmdtp);

-#ifdef CONFIG_AUTO_COMPLETE extern int var_complete(int argc, char * const argv[], char last_char, int maxv, char *cmdv[]); extern int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp); -#endif

/* * Monitor Command diff --git a/include/common.h b/include/common.h index f07f740..735258e 100644 --- a/include/common.h +++ b/include/common.h @@ -903,9 +903,7 @@ int pcmcia_init (void);

#include <bootstage.h>

-#ifdef CONFIG_SHOW_ACTIVITY void show_activity(int arg); -#endif

/* Multicore arch functions */ #ifdef CONFIG_MP