[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin
Before this commit, common/cmd_test.c defined _STDBOOL_H in order to avoid including <stdbool.h>. But this work-around is not a good idea.
Blackfin header file arch/blackfin/include/asm/blackfin_local.h uses bool type here: extern bool bfin_os_log_check(void);
This means Blackfin boards which define CONFIG_SYS_HUSH_PARSER always failed in compiling.
This commit fixes this issue by undefining true and false macro.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com ---
For example, when I try to compile bct-brettl2 board, I got an error like follows at compiling common/cmd_test.c.
bfin-uclinux-gcc -g -Os -ffixed-P3 -fomit-frame-pointer -mno-fdpic -ffunction-sections -fdata-sections -mcpu=bf536-0.3 -D__KERNEL__ -I/home/yamada/u-boot/include -I/home/yamada/u-boot/arch/blackfin/include -fno-builtin -ffreestanding -nostdinc -isystem /opt/gcc-4.6.3-nolibc/bfin-uclinux/bin/../lib/gcc/bfin-uclinux/4.6.3/include -pipe -DCONFIG_BLACKFIN -Wall -Wstrict-prototypes -fno-stack-protector -Wno-format-nonliteral -Wno-format-security -o cmd_test.o cmd_test.c -c In file included from /home/yamada/u-boot/arch/blackfin/include/asm/blackfin.h:13:0, from /home/yamada/u-boot/include/common.h:92, from cmd_test.c:17: /home/yamada/u-boot/arch/blackfin/include/asm/blackfin_local.h:54:1: error: unknown type name 'bool' make[2]: *** [cmd_test.o] Error 1 make[2]: Leaving directory `/home/yamada/u-boot/common' make[1]: *** [common/built-in.o] Error 2 make[1]: Leaving directory `/home/yamada/u-boot' make: *** [bct-brettl2] Error 2
This is not a compiler problem. It looks like this behavior is common for all blackfin compilers.
common/cmd_test.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/common/cmd_test.c b/common/cmd_test.c index bacc368..1800cff 100644 --- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -5,15 +5,6 @@ * SPDX-License-Identifier: GPL-2.0+ */
-/* - * Define _STDBOOL_H here to avoid macro expansion of true and false. - * If the future code requires macro true or false, remove this define - * and undef true and false before U_BOOT_CMD. This define and comment - * shall be removed if change to U_BOOT_CMD is made to take string - * instead of stringifying it. - */ -#define _STDBOOL_H - #include <common.h> #include <command.h>
@@ -143,6 +134,12 @@ U_BOOT_CMD( "[args..]" );
+/* + * Undef true and false here to avoid macro expansion by <stdbool.h> + */ +#undef true +#undef false + static int do_false(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { return 1;
FYI:
This patch is related with the discussion in the thread: http://patchwork.ozlabs.org/patch/292011/
Best Regards Masahiro Yamada
Dear Masahiro Yamada,
In message 1384830117-25345-1-git-send-email-yamada.m@jp.panasonic.com you wrote:
Before this commit, common/cmd_test.c defined _STDBOOL_H in order to avoid including <stdbool.h>. But this work-around is not a good idea.
Actually it is a good idea, as it attempts to be independent of the actual implementation of the bool data types - it does the same no matter if "true" and "flase" are members or a union or #define'd constants.
--- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -5,15 +5,6 @@
- SPDX-License-Identifier: GPL-2.0+
*/
-/*
- Define _STDBOOL_H here to avoid macro expansion of true and false.
- If the future code requires macro true or false, remove this define
- and undef true and false before U_BOOT_CMD. This define and comment
- shall be removed if change to U_BOOT_CMD is made to take string
- instead of stringifying it.
- */
-#define _STDBOOL_H
#include <common.h> #include <command.h>
@@ -143,6 +134,12 @@ U_BOOT_CMD( "[args..]" );
+/*
- Undef true and false here to avoid macro expansion by <stdbool.h>
- */
+#undef true +#undef false
I don't like this. I feel we should not change global files (that build fine for everyone else) to work around problems in one specific implementation. Instead, we should fix the problem at the root cause, for example like that. Could you please test if this patch fixes the problem, too?
From f68e524dd72c9cc08e86b479b82eff59ef6d591b Mon Sep 17 00:00:00 2001
From: Wolfgang Denk wd@denx.de Date: Tue, 19 Nov 2013 07:50:46 +0100 Subject: [PATCH] Blackfin: don't use 'bool' when it causes problems
The use of 'bool' data types in globally used header files cases build errors like this:
In file included from arch/blackfin/include/asm/blackfin.h:13:0, from include/common.h:92, from cmd_test.c:17: arch/blackfin/include/asm/blackfin_local.h:54:1: error: unknown type name 'bool'
Use plain 'int' instead to avoid such kind of trouble.
Signed-off-by: Wolfgang Denk wd@denx.de --- I always had the opinion that the use of "bool" types causes more problems than it solves. This case reinforces my opinion. - wd
arch/blackfin/cpu/os_log.c | 6 +++--- arch/blackfin/include/asm/blackfin_local.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/blackfin/cpu/os_log.c b/arch/blackfin/cpu/os_log.c index e1c8e29..2092d9e 100644 --- a/arch/blackfin/cpu/os_log.c +++ b/arch/blackfin/cpu/os_log.c @@ -12,12 +12,12 @@ #define OS_LOG_MAGIC_ADDR ((unsigned long *)0x4f0) #define OS_LOG_PTR_ADDR ((char **)0x4f4)
-bool bfin_os_log_check(void) +int bfin_os_log_check(void) { if (*OS_LOG_MAGIC_ADDR != OS_LOG_MAGIC) - return false; + return 0; *OS_LOG_MAGIC_ADDR = 0; - return true; + return 1; }
void bfin_os_log_dump(void) diff --git a/arch/blackfin/include/asm/blackfin_local.h b/arch/blackfin/include/asm/blackfin_local.h index ab31dcb..8ea8cde 100644 --- a/arch/blackfin/include/asm/blackfin_local.h +++ b/arch/blackfin/include/asm/blackfin_local.h @@ -51,7 +51,7 @@ extern u_long get_dclk(void);
# define bfin_revid() (bfin_read_CHIPID() >> 28)
-extern bool bfin_os_log_check(void); +extern int bfin_os_log_check(void); extern void bfin_os_log_dump(void);
extern void blackfin_icache_flush_range(const void *, const void *);
Hello Wolfgang
In message 1384830117-25345-1-git-send-email-yamada.m@jp.panasonic.com you wrote:
Before this commit, common/cmd_test.c defined _STDBOOL_H in order to avoid including <stdbool.h>. But this work-around is not a good idea.
Actually it is a good idea, as it attempts to be independent of the actual implementation of the bool data types - it does the same no matter if "true" and "flase" are members or a union or #define'd constants.
If you think so, the following also depends on the impilementation of <stdbool.h>, doesn't it?
#define _STDBOOL_H
#include <common.h> #include <command.h>
For example, if <stdbool.h> used _STDBOOL_H_ or __STDBOOL_H instead of _STDBOOL_H, <stdbool.h> would be included and common/cmd_test.c would not be compiled correctly.
I don't like this. I feel we should not change global files (that build fine for everyone else) to work around problems in one specific implementation. Instead, we should fix the problem at the root cause, for example like that. Could you please test if this patch fixes the problem, too?
From f68e524dd72c9cc08e86b479b82eff59ef6d591b Mon Sep 17 00:00:00 2001 From: Wolfgang Denk wd@denx.de Date: Tue, 19 Nov 2013 07:50:46 +0100 Subject: [PATCH] Blackfin: don't use 'bool' when it causes problems
[snipped]
@@ -51,7 +51,7 @@ extern u_long get_dclk(void);
# define bfin_revid() (bfin_read_CHIPID() >> 28)
-extern bool bfin_os_log_check(void); +extern int bfin_os_log_check(void); extern void bfin_os_log_dump(void);
extern void blackfin_icache_flush_range(const void *, const void *);
Yes. Your patch fixed the build error.
Best Regards Masahiro Yamada
Hello Wolfgang
Could you resend http://patchwork.ozlabs.org/patch/292309/ in a correct format?
(Or Tom can directly pick it from the ML?)
Best Regards Masahiro Yamada
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/24/2013 08:43 PM, Masahiro Yamada wrote:
Hello Wolfgang
Could you resend http://patchwork.ozlabs.org/patch/292309/ in a correct format?
(Or Tom can directly pick it from the ML?)
It's in my current local queue, I'll be pushing things later today.
- -- Tom
Dear Masahiro Yamada,
In message 20131125104322.C02E.AA925319@jp.panasonic.com you wrote:
Hello Wolfgang
Could you resend http://patchwork.ozlabs.org/patch/292309/ in a correct format?
(Or Tom can directly pick it from the ML?)
Of course I can. Does this patch solve the problem for you?
Best regards,
Wolfgang Denk
Hello Wolfgang
Could you resend http://patchwork.ozlabs.org/patch/292309/ in a correct format?
(Or Tom can directly pick it from the ML?)
Of course I can. Does this patch solve the problem for you?
Yes. It worked for me. Thanks. (And applied to u-boot/master)
It's OK for now because my motivation is to fix the build error on blackfin boards. Although it is true that I am a little concerned about this parts:
#define _STDBOOL_H
#include <common.h> #include <command.h>
Best Regards Masahiro Yamada
On Tue, Nov 19, 2013 at 08:01:44AM +0100, Wolfgang Denk wrote:
From: Wolfgang Denk wd@denx.de Date: Tue, 19 Nov 2013 07:50:46 +0100 Subject: [PATCH] Blackfin: don't use 'bool' when it causes problems
The use of 'bool' data types in globally used header files cases build errors like this:
In file included from arch/blackfin/include/asm/blackfin.h:13:0, from include/common.h:92, from cmd_test.c:17: arch/blackfin/include/asm/blackfin_local.h:54:1: error: unknown type name 'bool'
Use plain 'int' instead to avoid such kind of trouble.
Signed-off-by: Wolfgang Denk wd@denx.de
Applied to u-boot/master (and re-worded patchwork dl'd patch to match this), thanks!
Masahiro Yamada yamada.m@jp.panasonic.com writes:
Before this commit, common/cmd_test.c defined _STDBOOL_H in order to avoid including <stdbool.h>. But this work-around is not a good idea.
Blackfin header file arch/blackfin/include/asm/blackfin_local.h uses bool type here: extern bool bfin_os_log_check(void);
This means Blackfin boards which define CONFIG_SYS_HUSH_PARSER always failed in compiling.
This commit fixes this issue by undefining true and false macro.
[...]
+/*
- Undef true and false here to avoid macro expansion by <stdbool.h>
- */
+#undef true +#undef false
This won't work if stdbool.h defines true/false as an enum. Some versions of gcc did this. The correct fix is to change the code so it doesn't use any of the identifiers bool, true, or false for other purposes than those intended by stdbool.h.
I agree with Wolfgang that using "boolean" types is generally a bad idea that only introduces problems.
Hello Mans
+/*
- Undef true and false here to avoid macro expansion by <stdbool.h>
- */
+#undef true +#undef false
This won't work if stdbool.h defines true/false as an enum. Some versions of gcc did this. The correct fix is to change the code so it
Thanks for your comments. This patch does not work, so I'd like to retract it.
Best Regards Masahiro Yamada
participants (4)
-
Masahiro Yamada -
Måns Rullgård -
Tom Rini -
Wolfgang Denk