[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Masahiro Yamada]

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;
-- 
1.8.3.2

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Masahiro Yamada]

FYI:

This patch is related with the discussion in the thread:
http://patchwork.ozlabs.org/patch/292011/

Best Regards
Masahiro Yamada

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Wolfgang Denk]

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?

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Masahiro Yamada]

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

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[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

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Tom Rini]

-----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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSk0tuAAoJENk4IS6UOR1WE3YP/0cE/QOdExTnUwLqtGwoRlIX
bxugoR/7CfHF/o6bvv2Hh9KDXiydSeT7iXt3UauSBbUADC7jjDpBe6YFIteXVeew
xXzqx5fBoyEWHvSD66WyfSha06XQWj8isMn0AO/Tl1AfkYlBNbieSMhE6I1rGiC1
OoCxleTV1Ro5TDmVSf1ev0/K8h7uWXfYHbjX/EGqIoA65+nuUtlASp7Q2IOlZFEe
sTpGK07iPZEouFTt/KZhoV8uw3EERcW7AgaeYUUTYESYpYIAAI4w9PeejY7fOk01
38hXok93kvrLZqQ0HQn2pzVSd/FYCbh6Sx7o/FoTgtlmfHyEzoW/sVYCXj8lP82S
5mlXQwtIEDEFh7cnXYC4crOqyyGjyBrmpIVJ5QXGgS97ze/Vw6u/4iuoqqYg7uL+
voaYMeqi1UUs2dJFA9JO9+9bnzhPNRsf2cBcyf9keXm7GP1+IuoDw9BOX4e81Va0
XYPu8rJZ5GN20k4CAaoDG63PbYN42sR+rbERYJFe1HQ+ECtcuwVHN1xQdCHgOpUV
RYYpyC7WM7OK6TDQjuPAcEmGTioBWL/rz/dniFz1gvsFN0NEVEcPrf9l7JVloyEk
1mPUb4TsuOn0/rmcOXfsVtBmFac12ggy9Ci2xW1W06u9hA0e3EUGzYNxhMQPHoVT
5Y13IpA7dxCDYUHjMo7Q
=4mEj
-----END PGP SIGNATURE-----

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A right is not what someone gives you; it's what no one can take from
you.                                                   - Ramsey Clark

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[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?)
> 
> 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

[U-Boot] cmd_test: fix a compile error on Blackfin

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131125/1e31e493/attachment.pgp>

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com

[U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin

[Masahiro Yamada]

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