diff mbox

[OpenWrt-Devel] include/kernel.mk - better search for ARCH

Message ID 1438245719-14962-1-git-send-email-abrodkin@synopsys.com
State Changes Requested
Headers show

Commit Message

Alexey Brodkin July 30, 2015, 8:41 a.m. UTC
If "findstring" is used without leading and trailing spaces unexpected matches
may happen. For example consider ARC=arc then "findstring $(ARCH)" will
report a false match with "aarch64".

But "findstring $ARCH " (note trailing space) will correctly skip
matches for both "aarch64" and "aarch64_be".

This patch is built-tested against NetGear WNDR3800.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Jo-Philipp Wich <jow@openwrt.org>
---
 include/kernel.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Felix Fietkau July 30, 2015, 10:55 a.m. UTC | #1
On 2015-07-30 10:41, Alexey Brodkin wrote:
> If "findstring" is used without leading and trailing spaces unexpected matches
> may happen. For example consider ARC=arc then "findstring $(ARCH)" will
> report a false match with "aarch64".
> 
> But "findstring $ARCH " (note trailing space) will correctly skip
> matches for both "aarch64" and "aarch64_be".
> 
> This patch is built-tested against NetGear WNDR3800.
> 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Cc: Jo-Philipp Wich <jow@openwrt.org>
> ---
>  include/kernel.mk | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/kernel.mk b/include/kernel.mk
> index 7a0a170..95909fd 100644
> --- a/include/kernel.mk
> +++ b/include/kernel.mk
> @@ -62,15 +62,15 @@ endif
>  
>  ifneq (,$(findstring uml,$(BOARD)))
>    LINUX_KARCH=um
> -else ifneq (,$(findstring $(ARCH), aarch64 aarch64_be))
> +else ifneq (, $(findstring $(ARCH) , aarch64 aarch64_be ))
>    LINUX_KARCH := arm64
> -else ifneq (,$(findstring $(ARCH), armeb))
> +else ifneq (, $(findstring $(ARCH) , armeb ))
>    LINUX_KARCH := arm
> -else ifneq (,$(findstring $(ARCH), mipsel mips64 mips64el))
> +else ifneq (, $(findstring $(ARCH) , mipsel mips64 mips64el ))
>    LINUX_KARCH := mips
> -else ifneq (,$(findstring $(ARCH), sh2 sh3 sh4))
> +else ifneq (, $(findstring $(ARCH) , sh2 sh3 sh4 ))
>    LINUX_KARCH := sh
> -else ifneq (,$(findstring $(ARCH), i386 x86_64))
> +else ifneq (, $(findstring $(ARCH) , i386 x86_64 ))
Why did you add the leading whitespace before $(findstring...)?

- Felix
Alexey Brodkin July 30, 2015, 11:01 a.m. UTC | #2
Hi Felix,

On Thu, 2015-07-30 at 12:55 +0200, Felix Fietkau wrote:
> On 2015-07-30 10:41, Alexey Brodkin wrote:
> > If "findstring" is used without leading and trailing spaces unexpected matches
> > may happen. For example consider ARC=arc then "findstring $(ARCH)" will
> > report a false match with "aarch64".
> > 
> > But "findstring $ARCH " (note trailing space) will correctly skip
> > matches for both "aarch64" and "aarch64_be".
> > 
> > This patch is built-tested against NetGear WNDR3800.
> > 
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Felix Fietkau <nbd@openwrt.org>
> > Cc: Jo-Philipp Wich <jow@openwrt.org>
> > ---
> >  include/kernel.mk | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/kernel.mk b/include/kernel.mk
> > index 7a0a170..95909fd 100644
> > --- a/include/kernel.mk
> > +++ b/include/kernel.mk
> > @@ -62,15 +62,15 @@ endif
> >  
> >  ifneq (,$(findstring uml,$(BOARD)))
> >    LINUX_KARCH=um
> > -else ifneq (,$(findstring $(ARCH), aarch64 aarch64_be))
> > +else ifneq (, $(findstring $(ARCH) , aarch64 aarch64_be ))
> >    LINUX_KARCH := arm64
> > -else ifneq (,$(findstring $(ARCH), armeb))
> > +else ifneq (, $(findstring $(ARCH) , armeb ))
> >    LINUX_KARCH := arm
> > -else ifneq (,$(findstring $(ARCH), mipsel mips64 mips64el))
> > +else ifneq (, $(findstring $(ARCH) , mipsel mips64 mips64el ))
> >    LINUX_KARCH := mips
> > -else ifneq (,$(findstring $(ARCH), sh2 sh3 sh4))
> > +else ifneq (, $(findstring $(ARCH) , sh2 sh3 sh4 ))
> >    LINUX_KARCH := sh
> > -else ifneq (,$(findstring $(ARCH), i386 x86_64))
> > +else ifneq (, $(findstring $(ARCH) , i386 x86_64 ))
> Why did you add the leading whitespace before $(findstring...)?

Indeed this is not necessary because this is definitely out of
scope of "findstring".

If there're no more comments I'll send v2 soon.

-Alexey
diff mbox

Patch

diff --git a/include/kernel.mk b/include/kernel.mk
index 7a0a170..95909fd 100644
--- a/include/kernel.mk
+++ b/include/kernel.mk
@@ -62,15 +62,15 @@  endif
 
 ifneq (,$(findstring uml,$(BOARD)))
   LINUX_KARCH=um
-else ifneq (,$(findstring $(ARCH), aarch64 aarch64_be))
+else ifneq (, $(findstring $(ARCH) , aarch64 aarch64_be ))
   LINUX_KARCH := arm64
-else ifneq (,$(findstring $(ARCH), armeb))
+else ifneq (, $(findstring $(ARCH) , armeb ))
   LINUX_KARCH := arm
-else ifneq (,$(findstring $(ARCH), mipsel mips64 mips64el))
+else ifneq (, $(findstring $(ARCH) , mipsel mips64 mips64el ))
   LINUX_KARCH := mips
-else ifneq (,$(findstring $(ARCH), sh2 sh3 sh4))
+else ifneq (, $(findstring $(ARCH) , sh2 sh3 sh4 ))
   LINUX_KARCH := sh
-else ifneq (,$(findstring $(ARCH), i386 x86_64))
+else ifneq (, $(findstring $(ARCH) , i386 x86_64 ))
   LINUX_KARCH := x86
 else
   LINUX_KARCH := $(ARCH)