diff mbox series

[U-Boot] Enable expression support for CONFIG_BOARD_SIZE_LIMIT

Message ID 20181204154035.16299-1-wd@denx.de
State Changes Requested
Delegated to: Tom Rini
Headers show
Series [U-Boot] Enable expression support for CONFIG_BOARD_SIZE_LIMIT | expand

Commit Message

Wolfgang Denk Dec. 4, 2018, 3:40 p.m. UTC 
So far, the use of CONFIG_BOARD_SIZE_LIMIT would only work with
plain numeric constants.  Extend it to allow for expressions, so one
can for example use

	#define CONFIG_BOARD_SIZE_LIMIT	(768 << 10)

in the board configuration.

Signed-off-by: Wolfgang Denk <wd@denx.de>

Cc: Fabio Estevam <festevam@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Vanessa Maegima <vanessa.maegima@nxp.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: John Weber <john.weber@technexion.com>
Cc: Stefan Roese <sr@denx.de>

---

Note 1: As gawk lacks an eval function, we use bash's $((...))
	mechanism to evaluate the expression.  This has the
	additional benefit that it supports expressions like "<<"
	which awk does not understand.  OK, one could replace awk by
	something better...
Note 2: This patch focusses on enabling this new feature.  It does
	not addres another issue that should be solved in a lter
	commit: the duplication of the same code in Makefile and
	arch/arm/mach-imx/Makefile

 Makefile                   | 17 ++++++++---------
 arch/arm/mach-imx/Makefile | 17 ++++++++---------
 2 files changed, 16 insertions(+), 18 deletions(-)

Comments

Otavio Salvador Dec. 4, 2018, 3:42 p.m. UTC | #1 
On Tue, Dec 4, 2018 at 1:40 PM Wolfgang Denk <wd@denx.de> wrote:
>
> So far, the use of CONFIG_BOARD_SIZE_LIMIT would only work with
> plain numeric constants.  Extend it to allow for expressions, so one
> can for example use
>
>         #define CONFIG_BOARD_SIZE_LIMIT (768 << 10)
>
> in the board configuration.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Vanessa Maegima <vanessa.maegima@nxp.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: John Weber <john.weber@technexion.com>
> Cc: Stefan Roese <sr@denx.de>

Reviewed-by: Otavio Salvador <otavio@ossystems.com.br>

Thanks for looking into this.
Fabio Estevam Dec. 4, 2018, 4:15 p.m. UTC | #2 
Hi Wolfgang,

On Tue, Dec 4, 2018 at 1:40 PM Wolfgang Denk <wd@denx.de> wrote:
>
> So far, the use of CONFIG_BOARD_SIZE_LIMIT would only work with
> plain numeric constants.  Extend it to allow for expressions, so one
> can for example use
>
>         #define CONFIG_BOARD_SIZE_LIMIT (768 << 10)
>
> in the board configuration.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>

Still not working for me. I do see a warning now:

  LD      spl/u-boot-spl
/bin/sh: 1: arithmetic expression: expecting primary: ""((768 - 69) * 1024)""
  COPY    u-boot.bin
  MKIMAGE u-boot.img
  OBJCOPY spl/u-boot-spl-nodtb.bin
  COPY    spl/u-boot-spl.bin
  CFGS    spl/u-boot-spl.cfgout
  MKIMAGE SPL
  CFGCHK  u-boot.cfg

It does allow the build to proceed, but it is not really detecting the
overlap anymore.

For example: let's force the overlap by setting a very small
CONFIG_BOARD_SIZE_LIMIT of only 1K:

#define CONFIG_ENV_OFFSET (768 * 1024)
#define CONFIG_BOARD_SIZE_LIMIT (1 * 1024)

/bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""
  COPY    u-boot.bin
  MKIMAGE u-boot.img
  LD      spl/drivers/usb/gadget/built-in.o
  LD      spl/drivers/built-in.o
  LD      spl/u-boot-spl
  OBJCOPY spl/u-boot-spl-nodtb.bin
  COPY    spl/u-boot-spl.bin
  CFGS    spl/u-boot-spl.cfgout
  MKIMAGE SPL
  CFGCHK  u-boot.cfg

It still allowed a successful build, but it should have thrown an
error about the overlap.
Wolfgang Denk Dec. 5, 2018, 9:52 a.m. UTC | #3 
Dear Fabio,

In message <CAOMZO5Co8KPCBx+gPS8w02cYJR2Ci9cpSKVQm3zy+JRgD1mtLw@mail.gmail.com> you wrote:
>
> Still not working for me. I do see a warning now:
>
>   LD      spl/u-boot-spl
> /bin/sh: 1: arithmetic expression: expecting primary: ""((768 - 69) * 1024)""
>   COPY    u-boot.bin
>   MKIMAGE u-boot.img
>   OBJCOPY spl/u-boot-spl-nodtb.bin
>   COPY    spl/u-boot-spl.bin
>   CFGS    spl/u-boot-spl.cfgout
>   MKIMAGE SPL
>   CFGCHK  u-boot.cfg

I'm bulding with your modification:

diff --git a/include/configs/pico-imx7d.h b/include/configs/pico-imx7d.h
index 2bc42a04a0..67ca700a2f 100644
--- a/include/configs/pico-imx7d.h
+++ b/include/configs/pico-imx7d.h
@@ -134,7 +134,8 @@
 /* FLASH and environment organization */
 #define CONFIG_ENV_SIZE                        SZ_8K

-#define CONFIG_ENV_OFFSET                      (8 * SZ_64K)
+#define CONFIG_ENV_OFFSET              (768 * 1024)
+#define CONFIG_BOARD_SIZE_LIMIT                ((768 - 69) * 1024)
 #define CONFIG_SYS_FSL_USDHC_NUM               2

 #define CONFIG_SYS_MMC_ENV_DEV                 0

...
  LD      spl/common/spl/built-in.o
  CC      spl/lib/display_options.o
  LD      spl/lib/built-in.o
  LD      spl/u-boot-spl
  OBJCOPY spl/u-boot-spl-nodtb.bin
  COPY    spl/u-boot-spl.bin
  MKIMAGE SPL
  OBJCOPY u-boot.srec
  OBJCOPY u-boot-nodtb.bin
  COPY    u-boot.bin
  SYM     u-boot.sym
  MKIMAGE u-boot.img
  CHK     include/config.h
  CFG     u-boot.cfg
===================== WARNING ======================
This board does not use CONFIG_DM_MMC. Please update
the board to use CONFIG_DM_MMC before the v2019.04 release.
Failure to update by the deadline may result in board removal.
See doc/driver-model/MIGRATION.txt for more info.
====================================================
===================== WARNING ======================
This board does not use CONFIG_DM_USB. Please update
the board to use CONFIG_DM_USB before the v2019.07 release.
Failure to update by the deadline may result in board removal.
See doc/driver-model/MIGRATION.txt for more info.
====================================================
  CFGCHK  u-boot.cfg


> It does allow the build to proceed, but it is not really detecting the
> overlap anymore.
>
> For example: let's force the overlap by setting a very small
> CONFIG_BOARD_SIZE_LIMIT of only 1K:
>
> #define CONFIG_ENV_OFFSET (768 * 1024)
> #define CONFIG_BOARD_SIZE_LIMIT (1 * 1024)

Please try it on the shell:

-> echo $(( ((768 - 69) * 1024) ))
715776
-> echo $(( (1 * 1024) ))
1024

> /bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""

I tested this with /bin/bash, /bin/sh and even /bin/dash - they all
work fine here.

> It still allowed a successful build, but it should have thrown an
> error about the overlap.

It does for me, if I set the limit low:

  OBJCOPY spl/u-boot-spl-nodtb.bin
  COPY    spl/u-boot-spl.bin
  CFGS    spl/u-boot-spl.cfgout
  MKIMAGE SPL
  OBJCOPY u-boot.srec
  OBJCOPY u-boot-nodtb.bin
u-boot-nodtb.bin exceeds file size limit:
  limit:  1024 bytes
  actual: 480172 bytes
  excess: 479148 bytes
make: *** [Makefile:1071: u-boot-nodtb.bin] Error 1



Best regards,

Wolfgang Denk
Fabio Estevam Dec. 6, 2018, 1:04 p.m. UTC | #4 
Hi Wolfgang,

On Wed, Dec 5, 2018 at 7:52 AM Wolfgang Denk <wd@denx.de> wrote:

> Please try it on the shell:
>
> -> echo $(( ((768 - 69) * 1024) ))
> 715776
> -> echo $(( (1 * 1024) ))
> 1024

This works fine.

> It does for me, if I set the limit low:
>
>   OBJCOPY spl/u-boot-spl-nodtb.bin
>   COPY    spl/u-boot-spl.bin
>   CFGS    spl/u-boot-spl.cfgout
>   MKIMAGE SPL
>   OBJCOPY u-boot.srec
>   OBJCOPY u-boot-nodtb.bin
> u-boot-nodtb.bin exceeds file size limit:
>   limit:  1024 bytes
>   actual: 480172 bytes
>   excess: 479148 bytes
> make: *** [Makefile:1071: u-boot-nodtb.bin] Error 1

Previously I tested on a machine running Ubuntu 16.04 and today I also
tested on another machine running Ubuntu 18.04.

The results are the same on both machines:

1.  I get a "expecting primary" warning:

 OBJCOPY u-boot-nodtb.bin
/bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""
  SYM     u-boot.sym
  COPY    u-boot.bin
  MKIMAGE u-boot.img
  LD      spl/drivers/usb/host/built-in.o
  LD      spl/drivers/built-in.o
  LD      spl/u-boot-spl
  OBJCOPY spl/u-boot-spl-nodtb.bin
  COPY    spl/u-boot-spl.bin
  CFGS    spl/u-boot-spl.cfgout
  MKIMAGE SPL

2. The overlap is not detected anymore. In the example below I forced
#define CONFIG_BOARD_SIZE_LIMIT                (1 * 1024)
Wolfgang Denk Dec. 6, 2018, 2:23 p.m. UTC | #5 
Dear Fabio,

In message <CAOMZO5Aovrx43cmykUH+Bu_O_vC0CCs2hLVXFwCxmC2JDaKOYg@mail.gmail.com> you wrote:
>
> Previously I tested on a machine running Ubuntu 16.04 and today I also
> tested on another machine running Ubuntu 18.04.
>
> The results are the same on both machines:
>
> 1.  I get a "expecting primary" warning:
>
>  OBJCOPY u-boot-nodtb.bin
> /bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""

This makes really no sense to me.  Can you please send me (off list)
a git diff of the tree that is not building for you against the
nearest mainline commit?

And please also the output of /"bin/sh --version".

I have:

-> /bin/sh --version
GNU bash, version 4.4.23(1)-release (x86_64-redhat-linux-gnu)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.


[Sorry, can't test build under Ubuntu ATM; our container gives
strange errors, and I have to wait until tomorrow as I don't want to
mess with the setup.]

Best regards,

Wolfgang Denk
Fabio Estevam Dec. 6, 2018, 2:41 p.m. UTC | #6 
Hi Wolfgang,

On Thu, Dec 6, 2018 at 12:23 PM Wolfgang Denk <wd@denx.de> wrote:

> This makes really no sense to me.  Can you please send me (off list)
> a git diff of the tree that is not building for you against the
> nearest mainline commit?

I am running top of tree mainline U-Boot + your patch from this thread, plus:

--- a/include/configs/pico-imx7d.h
+++ b/include/configs/pico-imx7d.h
@@ -134,7 +134,8 @@
 /* FLASH and environment organization */
 #define CONFIG_ENV_SIZE                        SZ_8K

-#define CONFIG_ENV_OFFSET                      (8 * SZ_64K)
+#define CONFIG_ENV_OFFSET              (768 * 1024)
+#define CONFIG_BOARD_SIZE_LIMIT                (1 * 1024)
 #define CONFIG_SYS_FSL_USDHC_NUM               2

 #define CONFIG_SYS_MMC_ENV_DEV                 0

It does build fine, but as I have intentionally forced a small
CONFIG_BOARD_SIZE_LIMIT  I wanted it to fail, but it does not fail as
shown below:

  OBJCOPY u-boot-nodtb.bin
/bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""
  SYM     u-boot.sym
  COPY    u-boot.bin
  MKIMAGE u-boot.img
===================== WARNING ======================
This board does not use CONFIG_DM_MMC. Please update
the board to use CONFIG_DM_MMC before the v2019.04 release.
Failure to update by the deadline may result in board removal.
See doc/driver-model/MIGRATION.txt for more info.
====================================================
===================== WARNING ======================
This board does not use CONFIG_DM_USB. Please update
the board to use CONFIG_DM_USB before the v2019.07 release.
Failure to update by the deadline may result in board removal.
See doc/driver-model/MIGRATION.txt for more info.
====================================================
  CFGCHK  u-boot.cfg

As you can see there are two issues:

1. The warnin: /bin/sh: 1: arithmetic expression: expecting primary:
""(1 * 1024)""

2. It should have not built in this case. It should have detected the
overlap and signalized the error

> And please also the output of /"bin/sh --version".

$ /bin/sh --version
/bin/sh: 0: Illegal option --

In my system /bin/sh points to bash:
$ ls -al /bin/sh
lrwxrwxrwx 1 root root 4 mai  2  2018 /bin/sh -> dash

/bin/bash --version
GNU bash, version 4.4.19(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

I am able to reproduce this problem with a standalone script:

$ cat random.sh
#!/bin/bash
echo $(($RANDOM % 10))
$ ./random.sh
3

Worked fine with bash.

Now if I force it to dash:

$ cat random.sh
#!/bin/dash
echo $(($RANDOM % 10))
$ ./random.sh
./random.sh: 2: ./random.sh: arithmetic expression: expecting primary: " % 10"

Which is the same warning I get in U-Boot.
Andy Pont Dec. 6, 2018, 2:44 p.m. UTC | #7 
Fabio wrote...

>$ /bin/sh --version
>/bin/sh: 0: Illegal option --
>
>In my system /bin/sh points to bash:
>$ ls -al /bin/sh
>lrwxrwxrwx 1 root root 4 mai  2  2018 /bin/sh -> dash
Just to pick up on the obvious… /bin/sh is showing as linked to dash not 
bash!

Using (( … )) for arithmetic expansion is a bashism [1]

[1] https://wiki.ubuntu.com/DashAsBinSh

-Andy.
Philipp Tomsich Dec. 6, 2018, 2:50 p.m. UTC | #8 
Fabio,

let me chime in (I had to do a quick ‘git grep’ on this, as I couldn’t ignore
the mail traffic any longer)...

> On 06.12.2018, at 15:41, Fabio Estevam <festevam@gmail.com> wrote:
> 
> Hi Wolfgang,
> 
> On Thu, Dec 6, 2018 at 12:23 PM Wolfgang Denk <wd@denx.de> wrote:
> 
>> This makes really no sense to me.  Can you please send me (off list)
>> a git diff of the tree that is not building for you against the
>> nearest mainline commit?
> 
> I am running top of tree mainline U-Boot + your patch from this thread, plus:
> 
> --- a/include/configs/pico-imx7d.h
> +++ b/include/configs/pico-imx7d.h
> @@ -134,7 +134,8 @@
> /* FLASH and environment organization */
> #define CONFIG_ENV_SIZE                        SZ_8K
> 
> -#define CONFIG_ENV_OFFSET                      (8 * SZ_64K)
> +#define CONFIG_ENV_OFFSET              (768 * 1024)
> +#define CONFIG_BOARD_SIZE_LIMIT                (1 * 1024)

If you take a look at how CONFIG_BOARD_SIZE_LIMIT is used
	ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
	BOARD_SIZE_CHECK = \
	        @actual=`wc -c $@ | awk '{print $$1}'`; \
	        limit=`printf "%d" $(CONFIG_BOARD_SIZE_LIMIT)`; \
	        if test $$actual -gt $$limit; then \
	                echo "$@ exceeds file size limit:" >&2 ; \
	                echo "  limit:  $$limit bytes" >&2 ; \
	                echo "  actual: $$actual bytes" >&2 ; \
	                echo "  excess: $$((actual - limit)) bytes" >&2; \
	                exit 1; \
	        fi
	else
	BOARD_SIZE_CHECK =
	endif
you will notice that there’s no arithmetic expansion on it prior to it being
passed int a 'if -gt’ compare.

'git grep’ also shows that no other board is requesting an arithmetic
expansion on this (i.e. everyone else just uses a constant).

Note that the C-preprocessor will not do arithmetic for you...
So you’ll either have to change the Makefile or define this as an actual
constant number.

> #define CONFIG_SYS_FSL_USDHC_NUM               2
> 
> #define CONFIG_SYS_MMC_ENV_DEV                 0
> 
> It does build fine, but as I have intentionally forced a small
> CONFIG_BOARD_SIZE_LIMIT  I wanted it to fail, but it does not fail as
> shown below:
> 
>  OBJCOPY u-boot-nodtb.bin
> /bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""
>  SYM     u-boot.sym
>  COPY    u-boot.bin
>  MKIMAGE u-boot.img
> ===================== WARNING ======================
> This board does not use CONFIG_DM_MMC. Please update
> the board to use CONFIG_DM_MMC before the v2019.04 release.
> Failure to update by the deadline may result in board removal.
> See doc/driver-model/MIGRATION.txt for more info.
> ====================================================
> ===================== WARNING ======================
> This board does not use CONFIG_DM_USB. Please update
> the board to use CONFIG_DM_USB before the v2019.07 release.
> Failure to update by the deadline may result in board removal.
> See doc/driver-model/MIGRATION.txt for more info.
> ====================================================
>  CFGCHK  u-boot.cfg
> 
> As you can see there are two issues:
> 
> 1. The warnin: /bin/sh: 1: arithmetic expression: expecting primary:
> ""(1 * 1024)""
> 
> 2. It should have not built in this case. It should have detected the
> overlap and signalized the error
> 
>> And please also the output of /"bin/sh --version".
> 
> $ /bin/sh --version
> /bin/sh: 0: Illegal option --
> 
> In my system /bin/sh points to bash:
> $ ls -al /bin/sh
> lrwxrwxrwx 1 root root 4 mai  2  2018 /bin/sh -> dash
> 
> /bin/bash --version
> GNU bash, version 4.4.19(1)-release (x86_64-pc-linux-gnu)
> Copyright (C) 2016 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> 
> This is free software; you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> 
> I am able to reproduce this problem with a standalone script:
> 
> $ cat random.sh
> #!/bin/bash
> echo $(($RANDOM % 10))
> $ ./random.sh
> 3
> 
> Worked fine with bash.
> 
> Now if I force it to dash:
> 
> $ cat random.sh
> #!/bin/dash
> echo $(($RANDOM % 10))
> $ ./random.sh
> ./random.sh: 2: ./random.sh: arithmetic expression: expecting primary: " % 10"
> 
> Which is the same warning I get in U-Boot.
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Fabio Estevam Dec. 6, 2018, 2:58 p.m. UTC | #9 
On Thu, Dec 6, 2018 at 12:44 PM Andy Pont <andy.pont@sdcsystems.com> wrote:
>
> Fabio wrote...
>
> $ /bin/sh --version
> /bin/sh: 0: Illegal option --
>
> In my system /bin/sh points to bash:
> $ ls -al /bin/sh
> lrwxrwxrwx 1 root root 4 mai  2  2018 /bin/sh -> dash
>
> Just to pick up on the obvious… /bin/sh is showing as linked to dash not bash!

Yes, I meant bash.

>
> Using (( … )) for arithmetic expansion is a bashism [1]

Correct:
$ cat random.sh
#!/bin/sh
echo $(($RANDOM % 10))
$ checkbashisms random.sh
possible bashism in random.sh line 2 ($RANDOM):
echo $(($RANDOM % 10))
Fabio Estevam Dec. 6, 2018, 3:01 p.m. UTC | #10 
On Thu, Dec 6, 2018 at 12:58 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Thu, Dec 6, 2018 at 12:44 PM Andy Pont <andy.pont@sdcsystems.com> wrote:
> >
> > Fabio wrote...
> >
> > $ /bin/sh --version
> > /bin/sh: 0: Illegal option --
> >
> > In my system /bin/sh points to bash:
> > $ ls -al /bin/sh
> > lrwxrwxrwx 1 root root 4 mai  2  2018 /bin/sh -> dash
> >
> > Just to pick up on the obvious… /bin/sh is showing as linked to dash not bash!
>
> Yes, I meant bash.

I meant 'dash' :-)
Fabio Estevam Dec. 6, 2018, 3:06 p.m. UTC | #11 
Hi Philipp,

On Thu, Dec 6, 2018 at 12:50 PM Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:

> If you take a look at how CONFIG_BOARD_SIZE_LIMIT is used
>         ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
>         BOARD_SIZE_CHECK = \
>                 @actual=`wc -c $@ | awk '{print $$1}'`; \
>                 limit=`printf "%d" $(CONFIG_BOARD_SIZE_LIMIT)`; \
>                 if test $$actual -gt $$limit; then \
>                         echo "$@ exceeds file size limit:" >&2 ; \
>                         echo "  limit:  $$limit bytes" >&2 ; \
>                         echo "  actual: $$actual bytes" >&2 ; \
>                         echo "  excess: $$((actual - limit)) bytes" >&2; \
>                         exit 1; \
>                 fi
>         else
>         BOARD_SIZE_CHECK =
>         endif
> you will notice that there’s no arithmetic expansion on it prior to it being
> passed int a 'if -gt’ compare.

Yes, this the current code. The patch Wolfgang submitted in this
thread changed the Makefile.

>
> 'git grep’ also shows that no other board is requesting an arithmetic
> expansion on this (i.e. everyone else just uses a constant).
>
> Note that the C-preprocessor will not do arithmetic for you...
> So you’ll either have to change the Makefile or define this as an actual
> constant number.

Yes, Wolfgang's patch changed the Makefile to allow arithmetic
operation, but it does not work on my system.
Fabio Estevam Dec. 6, 2018, 3:17 p.m. UTC | #12 
Hi Wolfgang,

[Adding Andy]

On Thu, Dec 6, 2018 at 12:41 PM Fabio Estevam <festevam@gmail.com> wrote:

> I am running top of tree mainline U-Boot + your patch from this thread, plus:
>
> --- a/include/configs/pico-imx7d.h
> +++ b/include/configs/pico-imx7d.h
> @@ -134,7 +134,8 @@
>  /* FLASH and environment organization */
>  #define CONFIG_ENV_SIZE                        SZ_8K
>
> -#define CONFIG_ENV_OFFSET                      (8 * SZ_64K)
> +#define CONFIG_ENV_OFFSET              (768 * 1024)
> +#define CONFIG_BOARD_SIZE_LIMIT                (1 * 1024)
>  #define CONFIG_SYS_FSL_USDHC_NUM               2
>
>  #define CONFIG_SYS_MMC_ENV_DEV                 0
>
> It does build fine, but as I have intentionally forced a small
> CONFIG_BOARD_SIZE_LIMIT  I wanted it to fail, but it does not fail as
> shown below:
>
>   OBJCOPY u-boot-nodtb.bin
> /bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""

I read the link suggested by Andy Pont: https://wiki.ubuntu.com/DashAsBinSh

where it says:

"In Makefiles, you can set the following variable at the top:

SHELL = /bin/bash"

And by forcing the SHELL variable to bash, then your patch works fine here:

--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+

+SHELL = /bin/bash
 VERSION = 2019
 PATCHLEVEL = 01
 SUBLEVEL =
Wolfgang Denk Dec. 7, 2018, 3:21 p.m. UTC | #13 
Dear Fabio,

In message <CAOMZO5CnnmJrc9Y_9LQpxQYWQNcGeTUw=vgPtSpdmEPwavhU6g@mail.gmail.com> you wrote:
>
> > /bin/sh: 1: arithmetic expression: expecting primary: ""(1 * 1024)""

D*mn.  I really thought I had tried this in a dash based
environment, too.  Sorry for causing such confusion.

> SHELL = /bin/bash"

Yes, if this is really a bash only feature that would be an easy way
to fix it.

> And by forcing the SHELL variable to bash, then your patch works fine here:

Yes, this would work - but I'm not sure if everybody would appreciate
such a change?

This should also work - replace the line

	@(echo $$(($(CONFIG_BOARD_SIZE_LIMIT))); wc -c $@ ) | \

by

	@(awk "END { print $$(echo $(CONFIG_BOARD_SIZE_LIMIT)) }" /dev/null; wc -c $@ ) | \


Can you please try this out?

Best regards,

Wolfgang Denk
Fabio Estevam Dec. 7, 2018, 3:37 p.m. UTC | #14 
Hi Wolfgang,

On Fri, Dec 7, 2018 at 1:21 PM Wolfgang Denk <wd@denx.de> wrote:

> This should also work - replace the line
>
>         @(echo $$(($(CONFIG_BOARD_SIZE_LIMIT))); wc -c $@ ) | \
>
> by
>
>         @(awk "END { print $$(echo $(CONFIG_BOARD_SIZE_LIMIT)) }" /dev/null; wc -c $@ ) | \
>
>
> Can you please try this out?

I replaced it on the main Makefile and also in the imx one and it
works as expected now.

When you send the v2, you can add:

Tested-by: Fabio Estevam <festevam@gmail.com>

Thanks
Wolfgang Denk Dec. 7, 2018, 7:28 p.m. UTC | #15 
Dear Fabio,

In message <CAOMZO5BWH8g+h4pUBGzvgsS4Ae46kuREopx_0Fisyy7+KMq4Ug@mail.gmail.com> you wrote:
>
> I replaced it on the main Makefile and also in the imx one and it
> works as expected now.

Thanks.

> When you send the v2, you can add:
>
> Tested-by: Fabio Estevam <festevam@gmail.com>

Done.  Thanks for your patience.

Best regards,

Wolfgang Denk
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 0d11ff9797..d4c8f697cf 100644
--- a/Makefile
+++ b/Makefile
@@ -774,15 +774,14 @@  LDPPFLAGS += \
 
 ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
 BOARD_SIZE_CHECK = \
-	@actual=`wc -c $@ | awk '{print $$1}'`; \
-	limit=`printf "%d" $(CONFIG_BOARD_SIZE_LIMIT)`; \
-	if test $$actual -gt $$limit; then \
-		echo "$@ exceeds file size limit:" >&2 ; \
-		echo "  limit:  $$limit bytes" >&2 ; \
-		echo "  actual: $$actual bytes" >&2 ; \
-		echo "  excess: $$((actual - limit)) bytes" >&2; \
-		exit 1; \
-	fi
+        @(echo $$(($(CONFIG_BOARD_SIZE_LIMIT))); wc -c $@ ) | \
+	awk 'BEGIN { getline limit } \
+	{ if ($$1 > limit) { \
+		printf "%s exceeds file size limit:\n", $$2; \
+		printf "  limit:  %d bytes\n", limit; \
+		printf "  actual: %d bytes\n", $$1; \
+		printf "  excess: %d bytes\n", $$1 - limit; \
+		exit 1; } }'
 else
 BOARD_SIZE_CHECK =
 endif
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 53d9e5f42b..230a5c73aa 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -60,15 +60,14 @@  endif
 
 ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
 BOARD_SIZE_CHECK = \
-        @actual=`wc -c $@ | awk '{print $$1}'`; \
-        limit=`printf "%d" $(CONFIG_BOARD_SIZE_LIMIT)`; \
-        if test $$actual -gt $$limit; then \
-                echo "$@ exceeds file size limit:" >&2 ; \
-                echo "  limit:  $$limit bytes" >&2 ; \
-                echo "  actual: $$actual bytes" >&2 ; \
-                echo "  excess: $$((actual - limit)) bytes" >&2; \
-                exit 1; \
-        fi
+        @(echo $$(($(CONFIG_BOARD_SIZE_LIMIT))); wc -c $@ ) | \
+	awk 'BEGIN { getline limit } \
+	{ if ($$1 > limit) { \
+		printf "%s exceeds file size limit:\n", $$2; \
+		printf "  limit:  %d bytes\n", limit; \
+		printf "  actual: %d bytes\n", $$1; \
+		printf "  excess: %d bytes\n", $$1 - limit; \
+		exit 1; } }'
 else
 BOARD_SIZE_CHECK =
 endif