Skip to contentSkip to conversation

toochain/wrapper: fix potential bug in foreach loop

PATCH New Yann E. MORIN 3 participants 3 messages Oct 21, 2015 – Oct 25, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 toochain/wrapper: fix potential bug in foreach loop

In Makefile, the comma ',' is used to separate the arguments passed to functions, so we should not be allowed to use straight commas in strings we want to expand.

For the toolchain wrapper, we need to transform a list:
-mfoo -mbar -mbuz

into something acceptable for a C array assignment:

    "-mfoo", "-mbar", "-mbuz",

So, we use a $(foreach ...) loop for that. However, we do have a straight comma in there.

It does not cause any issue in practice, since $(foreach) is a make builtin function that accepts three and only three parameters.

However, this is not sane.

Change the straight comma to the usual $(comma) expansion, like we sould do for a call to any other function.

At the same time, make the code a bit easier to read, by first creating the transformed list, and then creating the define.

Cc Arnout Vandecappelle <[email protected]>
---
 toolchain/toolchain-wrapper.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
toolchain/toolchain-wrapper.mk +3 -1
index eba2b38..af39071 100644
@@ -14,7 +14,9 @@ TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
 
 # We create a list like '"-mfoo", "-mbar", "-mbarfoo"' so that each flag is a
 # separate argument when used in execv() by the toolchain wrapper.
-TOOLCHAIN_WRAPPER_ARGS += -DBR_ADDITIONAL_CFLAGS='$(foreach f,$(call qstrip,$(BR2_TARGET_OPTIMIZATION)),"$(f)",)'
+TOOLCHAIN_WRAPPER_OPTS = \
+	$(foreach f,$(call qstrip,$(BR2_TARGET_OPTIMIZATION)),"$(f)"$(comma))
+TOOLCHAIN_WRAPPER_ARGS += -DBR_ADDITIONAL_CFLAGS='$(TOOLCHAIN_WRAPPER_OPTS)'
 
 ifeq ($(BR2_CCACHE),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 toochain/wrapper: fix potential bug in foreach loop

Quoted 18 lines · Yann E. MORIN

In Makefile, the comma ',' is used to separate the arguments passed to functions, so we should not be allowed to use straight commas in strings we want to expand.

For the toolchain wrapper, we need to transform a list:
-mfoo -mbar -mbuz

into something acceptable for a C array assignment:

    "-mfoo", "-mbar", "-mbuz",

So, we use a $(foreach ...) loop for that. However, we do have a straight comma in there.

It does not cause any issue in practice, since $(foreach) is a make builtin function that accepts three and only three parameters.

However, this is not sane.

Quoted 8 lines

Change the straight comma to the usual $(comma) expansion, like we sould do for a call to any other function.

At the same time, make the code a bit easier to read, by first creating the transformed list, and then creating the define.

Cc Arnout Vandecappelle <[email protected]>

(Build test with BR2_TARGET_OPTIMIZATION containing two elements)

Regards,
Arnout

Quoted 20 lines
---
 toolchain/toolchain-wrapper.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
toolchain/toolchain-wrapper.mk +3 -1
index eba2b38..af39071 100644
@@ -14,7 +14,9 @@ TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
 
 # We create a list like '"-mfoo", "-mbar", "-mbarfoo"' so that each flag is a
 # separate argument when used in execv() by the toolchain wrapper.
-TOOLCHAIN_WRAPPER_ARGS += -DBR_ADDITIONAL_CFLAGS='$(foreach f,$(call qstrip,$(BR2_TARGET_OPTIMIZATION)),"$(f)",)'
+TOOLCHAIN_WRAPPER_OPTS = \
+	$(foreach f,$(call qstrip,$(BR2_TARGET_OPTIMIZATION)),"$(f)"$(comma))
+TOOLCHAIN_WRAPPER_ARGS += -DBR_ADDITIONAL_CFLAGS='$(TOOLCHAIN_WRAPPER_OPTS)'
 
 ifeq ($(BR2_CCACHE),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#2 lore raw

PATCH 1/1 toochain/wrapper: fix potential bug in foreach loop

In reply to Yann E. MORIN
Quoted 20 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

In Makefile, the comma ',' is used to separate the arguments passed to functions, so we should not be allowed to use straight commas in strings we want to expand.

For the toolchain wrapper, we need to transform a list:
-mfoo -mbar -mbuz

into something acceptable for a C array assignment:

    "-mfoo", "-mbar", "-mbuz",

So, we use a $(foreach ...) loop for that. However, we do have a straight comma in there.

It does not cause any issue in practice, since $(foreach) is a make builtin function that accepts three and only three parameters.

However, this is not sane.

Quoted 8 lines

Change the straight comma to the usual $(comma) expansion, like we sould do for a call to any other function.

At the same time, make the code a bit easier to read, by first creating the transformed list, and then creating the define.

Cc Arnout Vandecappelle <[email protected]>

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard