* [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]"
@ 2013-06-13 13:10 Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 1/3] h8300/boot: " Geert Uytterhoeven
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2013-06-13 13:10 UTC (permalink / raw)
To: linux-arm-kernel
This patch series replaces the bash-specific "$[...]" by POSIX "$((..))"
for arithmetic expansion, and gets rid of some dependencies on bash.
[1/3] h8300/boot: Use POSIX "$((..))" instead of bashism "$[...]"
[2/3] ARM: shmobile: Use POSIX "$((..))" instead of bashism "$[...]"
[3/3] sh/boot: Use POSIX "$((..))" instead of bashism "$[...]"
The first one is a build fix on systems where /bin/sh is not bash, as it
uses the bashism without forcing bash.
This series was tested using the patch at the bottom, and running
make ARCH=h8300 clean
make ARCH=arm clean
make ARCH=sh clean
with an optional "SHELL=/bin/..." appended, to test difference shells,
on Ubuntu 10.04.4 LTS.
It works fine with bash (4.1-2ubuntu3) and dash (0.5.5.1-3ubuntu2).
With mksh (39.3-1ubuntu3), 32-bit integers seem to be sign-extended to
64-bit integers, e.g.
arch/sh/boot/Makefile: KERNEL_MEMORY = 0x0c000000
arch/sh/boot/Makefile: KERNEL_LOAD = 0xffffffff8c001000
arch/sh/boot/Makefile: KERNEL_ENTRY = 0xffffffff8c002000
arch/sh/boot/compressed/Makefile: IMAGE_OFFSET = 0xffffffff8c800000
Should these be truncated to 32-bit explicitly, or is this a bug in mksh?
Thanks for your comments!
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 1ba358b..5029108 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -319,3 +319,5 @@ define archhelp
echo ' (distribution) /sbin/$(INSTALLKERNEL) or'
echo ' install to $$(INSTALL_PATH) and run lilo'
endef
+
+include arch/arm/mach-shmobile/Makefile.boot
diff --git a/arch/arm/mach-shmobile/Makefile.boot b/arch/arm/mach-shmobile/Makefile.boot
index 498efd9..95ebc7c 100644
--- a/arch/arm/mach-shmobile/Makefile.boot
+++ b/arch/arm/mach-shmobile/Makefile.boot
@@ -7,3 +7,5 @@ __ZRELADDR := $(shell /bin/bash -c 'printf "0x%08x" \
#
#params_phys-y (Instead: Pass atags pointer in r2)
#initrd_phys-y (Instead: Use compiled-in initramfs)
+
+XXX := $(shell echo arch/arm/mach-shmobile/Makefile.boot: __ZRELADDR = $(__ZRELADDR) >> /tmp/loggy)
diff --git a/arch/h8300/boot/compressed/Makefile b/arch/h8300/boot/compressed/Makefile
index 6745cb1..94faad1 100644
--- a/arch/h8300/boot/compressed/Makefile
+++ b/arch/h8300/boot/compressed/Makefile
@@ -35,3 +35,5 @@ OBJCOPYFLAGS := -O binary
$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.gz FORCE
$(call if_changed,ld)
+
+XXX := $(shell echo arch/h8300/boot/compressed/Makefile: IMAGE_OFFSET = $(IMAGE_OFFSET) >> /tmp/loggy)
diff --git a/arch/sh/boot/Makefile b/arch/sh/boot/Makefile
index 58592df..4ab2ba5 100644
--- a/arch/sh/boot/Makefile
+++ b/arch/sh/boot/Makefile
@@ -113,3 +113,7 @@ $(obj)/uImage: $(obj)/uImage.$(suffix-y)
export CONFIG_PAGE_OFFSET CONFIG_MEMORY_START CONFIG_BOOT_LINK_OFFSET \
CONFIG_PHYSICAL_START CONFIG_ZERO_PAGE_OFFSET CONFIG_ENTRY_OFFSET \
KERNEL_MEMORY suffix-y
+
+XXX := $(shell echo arch/sh/boot/Makefile: KERNEL_MEMORY = $(KERNEL_MEMORY) >> /tmp/loggy)
+XXX := $(shell echo arch/sh/boot/Makefile: KERNEL_LOAD = $(KERNEL_LOAD) >> /tmp/loggy)
+XXX := $(shell echo arch/sh/boot/Makefile: KERNEL_ENTRY = $(KERNEL_ENTRY) >> /tmp/loggy)
diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
index 23bc849..43d1b9a 100644
--- a/arch/sh/boot/compressed/Makefile
+++ b/arch/sh/boot/compressed/Makefile
@@ -79,3 +79,5 @@ LDFLAGS_piggy.o := -r --format binary --oformat $(ld-bfd) -T
$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
$(call if_changed,ld)
+
+XXX := $(shell echo arch/sh/boot/compressed/Makefile: IMAGE_OFFSET = $(IMAGE_OFFSET) >> /tmp/loggy)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
In-Reply-To:
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 1/3] h8300/boot: Use POSIX "$((..))" instead of bashism "$[...]"
2013-06-13 13:10 [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]" Geert Uytterhoeven
@ 2013-06-13 13:10 ` Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 2/3] ARM: shmobile: " Geert Uytterhoeven
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2013-06-13 13:10 UTC (permalink / raw)
To: linux-arm-kernel
On Ubuntu, where /bin/sh -> dash, "make ARCH=h8300 clean" gives:
printf: 1: $[0x00400000+0x00140000]: expected numeric value
Replace the bash-specific "$[...]" by POSIX "$((..))" for arithmetic
expansion to fix this.
Note that according to the bash 4.1 manpage, "$[...]" is deprecated, and
will be removed in upcoming versions of bash.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
arch/h8300/boot/compressed/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/h8300/boot/compressed/Makefile b/arch/h8300/boot/compressed/Makefile
index 94faad1..787fd04 100644
--- a/arch/h8300/boot/compressed/Makefile
+++ b/arch/h8300/boot/compressed/Makefile
@@ -16,7 +16,7 @@ OBJECTS = $(obj)/head.o $(obj)/misc.o
#
CONFIG_MEMORY_START ?= 0x00400000
CONFIG_BOOT_LINK_OFFSET ?= 0x00140000
-IMAGE_OFFSET := $(shell printf "0x%08x" $$[$(CONFIG_MEMORY_START)+$(CONFIG_BOOT_LINK_OFFSET)])
+IMAGE_OFFSET := $(shell printf "0x%08x" $$(($(CONFIG_MEMORY_START)+$(CONFIG_BOOT_LINK_OFFSET))))
LDFLAGS_vmlinux := -Ttext $(IMAGE_OFFSET) -estartup $(obj)/vmlinux.lds
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ARM: shmobile: Use POSIX "$((..))" instead of bashism "$[...]"
2013-06-13 13:10 [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]" Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 1/3] h8300/boot: " Geert Uytterhoeven
@ 2013-06-13 13:10 ` Geert Uytterhoeven
2013-06-13 13:29 ` Simon Horman
2013-06-13 13:10 ` [PATCH 3/3] sh/boot: " Geert Uytterhoeven
2013-06-13 19:16 ` [PATCH 0/3] " Thorsten Glaser
3 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2013-06-13 13:10 UTC (permalink / raw)
To: linux-arm-kernel
According to the bash 4.1 manpage, "$[...]" is deprecated, and will be
removed in upcoming versions of bash.
Hence replace the bash-specific "$[...]" by POSIX "$((..))" for arithmetic
expansion, which also allows to drop the forced use of bash.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
arch/arm/mach-shmobile/Makefile.boot | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-shmobile/Makefile.boot b/arch/arm/mach-shmobile/Makefile.boot
index 95ebc7c..75a55e1 100644
--- a/arch/arm/mach-shmobile/Makefile.boot
+++ b/arch/arm/mach-shmobile/Makefile.boot
@@ -1,5 +1,5 @@
-__ZRELADDR := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_MEMORY_START) + 0x8000]')
+__ZRELADDR := $(shell printf "0x%08x" \
+ $$(($(CONFIG_MEMORY_START) + 0x8000)))
zreladdr-y += $(__ZRELADDR)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ARM: shmobile: Use POSIX "$((..))" instead of bashism "$[...]"
2013-06-13 13:10 ` [PATCH 2/3] ARM: shmobile: " Geert Uytterhoeven
@ 2013-06-13 13:29 ` Simon Horman
0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2013-06-13 13:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 13, 2013 at 03:10:51PM +0200, Geert Uytterhoeven wrote:
> According to the bash 4.1 manpage, "$[...]" is deprecated, and will be
> removed in upcoming versions of bash.
> Hence replace the bash-specific "$[...]" by POSIX "$((..))" for arithmetic
> expansion, which also allows to drop the forced use of bash.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> arch/arm/mach-shmobile/Makefile.boot | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/Makefile.boot b/arch/arm/mach-shmobile/Makefile.boot
> index 95ebc7c..75a55e1 100644
> --- a/arch/arm/mach-shmobile/Makefile.boot
> +++ b/arch/arm/mach-shmobile/Makefile.boot
> @@ -1,5 +1,5 @@
> -__ZRELADDR := $(shell /bin/bash -c 'printf "0x%08x" \
> - $$[$(CONFIG_MEMORY_START) + 0x8000]')
> +__ZRELADDR := $(shell printf "0x%08x" \
> + $$(($(CONFIG_MEMORY_START) + 0x8000)))
>
> zreladdr-y += $(__ZRELADDR)
This code was (very) recently changed by a patch that I
have queued up for v3.11 in the cleanup-boot branch of my renesas tree
on kernel.org. I believe that patch removes the instance of $[..]].
commit 12dca809ef785e451263351325d4806198040b40
Author: Magnus Damm <damm@opensource.se>
Date: Mon Jun 10 18:28:57 2013 +0900
ARM: shmobile: uImage load address rework
This is V2 of the mach-shmobile uImage load address rework patch.
Rework the mach-shmobile uImage load address calculation by storing
the per-board load addresses in Makefile.boot. This removes the
CONFIG_MEMORY_START dependency from Makefile.boot, and it also makes
it possible to create safe kernel images that boot on multiple boards.
This is one of several series of code that reworks code not to rely on
CONFIG_MEMORY_START/SIZE which in turn is needed for ARCH_MULTIPLATFORM.
Signed-off-by: Magnus Damm <damm@opensource.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
diff --git a/arch/arm/mach-shmobile/Makefile.boot b/arch/arm/mach-shmobile/Makefile.boot
index 498efd9..6b147ea 100644
--- a/arch/arm/mach-shmobile/Makefile.boot
+++ b/arch/arm/mach-shmobile/Makefile.boot
@@ -1,6 +1,22 @@
-__ZRELADDR := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_MEMORY_START) + 0x8000]')
+# per-board load address for uImage
+loadaddr-y :=
+loadaddr-$(CONFIG_MACH_AG5EVM) += 0x40008000
+loadaddr-$(CONFIG_MACH_AP4EVB) += 0x40008000
+loadaddr-$(CONFIG_MACH_APE6EVM) += 0x40008000
+loadaddr-$(CONFIG_MACH_ARMADILLO800EVA) += 0x40008000
+loadaddr-$(CONFIG_MACH_ARMADILLO800EVA_REFERENCE) += 0x40008000
+loadaddr-$(CONFIG_MACH_BOCKW) += 0x60008000
+loadaddr-$(CONFIG_MACH_BONITO) += 0x40008000
+loadaddr-$(CONFIG_MACH_KOTA2) += 0x41008000
+loadaddr-$(CONFIG_MACH_KZM9D) += 0x40008000
+loadaddr-$(CONFIG_MACH_KZM9G) += 0x41008000
+loadaddr-$(CONFIG_MACH_KZM9G_REFERENCE) += 0x41008000
+loadaddr-$(CONFIG_MACH_LAGER) += 0x40008000
+loadaddr-$(CONFIG_MACH_MACKEREL) += 0x40008000
+loadaddr-$(CONFIG_MACH_MARZEN) += 0x60008000
+loadaddr-$(CONFIG_MACH_MARZEN_REFERENCE) += 0x60008000
+__ZRELADDR := $(sort $(loadaddr-y))
zreladdr-y += $(__ZRELADDR)
# Unsupported legacy stuff
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] sh/boot: Use POSIX "$((..))" instead of bashism "$[...]"
2013-06-13 13:10 [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]" Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 1/3] h8300/boot: " Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 2/3] ARM: shmobile: " Geert Uytterhoeven
@ 2013-06-13 13:10 ` Geert Uytterhoeven
2013-06-13 19:16 ` [PATCH 0/3] " Thorsten Glaser
3 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2013-06-13 13:10 UTC (permalink / raw)
To: linux-arm-kernel
According to the bash 4.1 manpage, "$[...]" is deprecated, and will be
removed in upcoming versions of bash.
Hence replace the bash-specific "$[...]" by POSIX "$((..))" for arithmetic
expansion, which also allows to drop the forced use of bash.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
arch/sh/boot/Makefile | 24 ++++++++++++------------
arch/sh/boot/compressed/Makefile | 14 +++++++-------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/sh/boot/Makefile b/arch/sh/boot/Makefile
index 4ab2ba5..e10fb9b 100644
--- a/arch/sh/boot/Makefile
+++ b/arch/sh/boot/Makefile
@@ -46,18 +46,18 @@ $(obj)/romImage: $(obj)/romimage/vmlinux FORCE
$(obj)/romimage/vmlinux: $(obj)/zImage FORCE
$(Q)$(MAKE) $(build)=$(obj)/romimage $@
-KERNEL_MEMORY := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_PHYSICAL_START) & 0x1fffffff]')
-
-KERNEL_LOAD := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_PAGE_OFFSET) + \
- $(KERNEL_MEMORY) + \
- $(CONFIG_ZERO_PAGE_OFFSET)]')
-
-KERNEL_ENTRY := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_PAGE_OFFSET) + \
- $(KERNEL_MEMORY) + \
- $(CONFIG_ZERO_PAGE_OFFSET) + $(CONFIG_ENTRY_OFFSET)]')
+KERNEL_MEMORY := $(shell printf "0x%08x" \
+ $$(($(CONFIG_PHYSICAL_START) & 0x1fffffff)))
+
+KERNEL_LOAD := $(shell printf "0x%08x" \
+ $$(($(CONFIG_PAGE_OFFSET) + \
+ $(KERNEL_MEMORY) + \
+ $(CONFIG_ZERO_PAGE_OFFSET))))
+
+KERNEL_ENTRY := $(shell printf "0x%08x" \
+ $$(($(CONFIG_PAGE_OFFSET) + \
+ $(KERNEL_MEMORY) + \
+ $(CONFIG_ZERO_PAGE_OFFSET) + $(CONFIG_ENTRY_OFFSET))))
UIMAGE_LOADADDR = $(KERNEL_LOAD)
UIMAGE_ENTRYADDR = $(KERNEL_ENTRY)
diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
index 43d1b9a..0330ba7 100644
--- a/arch/sh/boot/compressed/Makefile
+++ b/arch/sh/boot/compressed/Makefile
@@ -17,14 +17,14 @@ GCOV_PROFILE := n
# IMAGE_OFFSET is the load offset of the compression loader
#
ifeq ($(CONFIG_32BIT),y)
-IMAGE_OFFSET := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_MEMORY_START) + \
- $(CONFIG_BOOT_LINK_OFFSET)]')
+IMAGE_OFFSET := $(shell printf "0x%08x" \
+ $$(($(CONFIG_MEMORY_START) + \
+ $(CONFIG_BOOT_LINK_OFFSET))))
else
-IMAGE_OFFSET := $(shell /bin/bash -c 'printf "0x%08x" \
- $$[$(CONFIG_PAGE_OFFSET) + \
- $(KERNEL_MEMORY) + \
- $(CONFIG_BOOT_LINK_OFFSET)]')
+IMAGE_OFFSET := $(shell printf "0x%08x" \
+ $$(($(CONFIG_PAGE_OFFSET) + \
+ $(KERNEL_MEMORY) + \
+ $(CONFIG_BOOT_LINK_OFFSET))))
endif
ifeq ($(CONFIG_MCOUNT),y)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]"
2013-06-13 13:10 [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]" Geert Uytterhoeven
` (2 preceding siblings ...)
2013-06-13 13:10 ` [PATCH 3/3] sh/boot: " Geert Uytterhoeven
@ 2013-06-13 19:16 ` Thorsten Glaser
3 siblings, 0 replies; 6+ messages in thread
From: Thorsten Glaser @ 2013-06-13 19:16 UTC (permalink / raw)
To: linux-arm-kernel
Geert Uytterhoeven dixit:
>Should these be truncated to 32-bit explicitly, or is this a bug in mksh?
mksh in ?mksh mode? operates specifically on 32-bit integer data
types with defined wraparound and other guarantees beyond what
POSIX does. There is an ?lksh? binary in the mksh binary package
in Debian and derivates now, which is the ?legacy mode? that uses
the host system?s ?long? data type, as POSIX demands, and is now
(mksh_46-2) used for /bin/sh.
However, on mixed 32/64-bit systems, /bin/sh can have either bit
size. Additionally, it is not possible, in POSIX, without invoking
ISO C ?Undefined Behaviour?, to find out the bitsize of the shell
arithmetics.
Furthermore, Linux can be cross-compiled, so when building kernels
for 64-bit platforms on 32-bit systems, the arithmetics used MUST
NOT overflow beyond a signed 32-bit ?long?.
> [1/3] h8300/boot: Use POSIX "$((..))" instead of bashism "$[...]"
> [2/3] ARM: shmobile: Use POSIX "$((..))" instead of bashism "$[...]"
> [3/3] sh/boot: Use POSIX "$((..))" instead of bashism "$[...]"
Independent of the above, I?ve verified all three and can state
that they
? are no regression relative to existing behaviour
? do not invoke any features not in POSIX $((?)) arithmetics
? do not invoke any features not in mksh $((?)) arithmetics
This means you can add my
Signed-off: Thorsten Glaser <tg@mirbsd.org>
However, I urge you to check whether any of these arithmetics
can go beyond 32 bit. If they have even the slightest chance
to do that, you MUST replace them by something different. One
method could be to use bc(1):
$(shell printf 'obase=16\nibase=16\n%s+%s\n' $(FOO) $(BAR) | bc)
Another method could be to operate on the upper half and the
lower half of the 64-bit quantities separately, assuming that
calculations do not overflow (in POSIX sh, overflow is, like
in ISO C, Undefined Behaviour; a C compiler is permitted to
compile the source code in the shell that invokes it to run
?rm -rf ~ /? instead) or carry over (e.g. if it?s known that
the first half is always 0x7FFFFFFF or the last half always
00000000 you can just add that as ?string?).
bye,
//mirabilos
--
FWIW, I'm quite impressed with mksh interactively. I thought it was much
*much* more bare bones. But it turns out it beats the living hell out of
ksh93 in that respect. I'd even consider it for my daily use if I hadn't
wasted half my life on my zsh setup. :-) -- Frank Terbeck in #!/bin/mksh
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-13 19:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-13 13:10 [PATCH 0/3] Use POSIX "$((..))" instead of bashism "$[...]" Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 1/3] h8300/boot: " Geert Uytterhoeven
2013-06-13 13:10 ` [PATCH 2/3] ARM: shmobile: " Geert Uytterhoeven
2013-06-13 13:29 ` Simon Horman
2013-06-13 13:10 ` [PATCH 3/3] sh/boot: " Geert Uytterhoeven
2013-06-13 19:16 ` [PATCH 0/3] " Thorsten Glaser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).