Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error

2014-10-19 Thread Bastian Bittorf 
* Nathan Hintz nlhi...@hotmail.com [19.10.2014 08:49]:
  case $leddc in
  '0x'*)
  leddc='0x005a000a'
  ;;
  esac
  
  IMHO the approach from felix does not work:
  root@box:~ echo $((0x))
  65535
  root@box:~ echo $((0x\n))
  -ash: arithmetic syntax error
  
  the real would be the fix the output of 'wlc'.
 
 'wlc' is not broken, a simple test case demonstrates it works as expected:
 
 root@e3000-1:~# wlc ifname wl0 leddc
 0x005a000a
 root@e3000-1:~# wlc ifname wl0 leddc 0x
 root@e3000-1:~# wlc ifname wl0 leddc
 0x
 root@e3000-1:~# leddc=$(wlc ifname wl0 leddc)
 root@e3000-1:~# [ $leddc = 0x ]  echo yes
 yes
 root@e3000-1:~# wlc ifname wl0 leddc 0x005a000a
 root@e3000-1:~# wlc ifname wl0 leddc
 0x005a000a
 root@e3000-1:~# leddc=$(wlc ifname wl0 leddc)
 root@e3000-1:~# [ $leddc = 0x ]  echo yes
 root@e3000-1:~#

ah - ok. sorry felix - i really was thinking that it outputs
a string '0x\n' (with the symbol of a newline). so your
approach is correct but without quotes, so:

[ $((leddc)) -eq $((0x)) ]  ...

bye, bastian - please Álvaro send a new patch
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error

2014-10-18 Thread Felix Fietkau 
On 2014-10-18 13:55, Álvaro Fernández Rojas wrote:
 wlc returns a string number ending with \n, making it impossible to compare 
 this value to a number.
 
 Signed-off-by: Álvaro Fernández Rojas nolt...@gmail.com
 ---
 v4: remove bashishm as suggested by Bastian Bittorf.
 v3: avoid using an extra variable.
 v2: use string comparison
 
 diff --git a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh 
 b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
 index 69e3132..5994e26 100644
 --- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
 +++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
 @@ -209,7 +209,7 @@ enable_broadcom() {
   }
  
   local leddc=$(wlc ifname $device leddc)
 - [ $leddc -eq 0x ] || {
 + [ $leddc = '0x\n' ] || {
I don't like this comparison against a fixed string with \n.
I'd prefer something like: [ $(($leddc)) -eq $((0x)) ]

- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error

2014-10-18 Thread Nathan Hintz 



Hi Alvaro:

 Date: Sat, 18 Oct 2014 13:55:39 +0200
 From: nolt...@gmail.com
 To: openwrt-devel@lists.openwrt.org; ha...@hauke-m.de; blo...@openwrt.org; 
 n...@openwrt.org
 Subject: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error
 
 wlc returns a string number ending with \n, making it impossible to compare 
 this value to a number.
 
 Signed-off-by: Álvaro Fernández Rojas nolt...@gmail.com
 ---
 v4: remove bashishm as suggested by Bastian Bittorf.
 v3: avoid using an extra variable.
 v2: use string comparison
 
 diff --git a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh 
 b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
 index 69e3132..5994e26 100644
 --- a/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
 +++ b/package/kernel/broadcom-wl/files/lib/wifi/broadcom.sh
 @@ -209,7 +209,7 @@ enable_broadcom() {
   }
  
   local leddc=$(wlc ifname $device leddc)
 - [ $leddc -eq 0x ] || {
 + [ $leddc = '0x\n' ] || {
   leddc=0x005a000a;
   }
Using the \n is not correct.  I think the real problem is that the logic is 
reversed (should be  instead of ||); although it might
be better to eliminate the conditional entirely since leddc is always commanded 
to 0x when the interface is taken down.

local leddc=$(wlc ifname $device leddc)
[ $leddc = 0x ]  {
leddc=0x005a000a
}

Nathan

  
 ___
 openwrt-devel mailing list
 openwrt-devel@lists.openwrt.org
 https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

  ___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error

2014-10-18 Thread Bastian Bittorf 
* Nathan Hintz nlhi...@hotmail.com [18.10.2014 22:24]:
 Using the \n is not correct.  I think the real problem is that the logic is 
 reversed (should be  instead of ||); although it might
 be better to eliminate the conditional entirely since leddc is always 
 commanded to 0x when the interface is taken down.
 
 local leddc=$(wlc ifname $device leddc)
 [ $leddc = 0x ]  {
   leddc=0x005a000a
 }

what about:

case $leddc in
'0x'*)
leddc='0x005a000a'
;;
esac

IMHO the approach from felix does not work:
root@box:~ echo $((0x))
65535
root@box:~ echo $((0x\n))
-ash: arithmetic syntax error

the real would be the fix the output of 'wlc'.

bye bastian
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error

2014-10-18 Thread Felix Fietkau 
On 2014-10-18 22:37, Bastian Bittorf wrote:
 * Nathan Hintz nlhi...@hotmail.com [18.10.2014 22:24]:
 Using the \n is not correct.  I think the real problem is that the logic 
 is reversed (should be  instead of ||); although it might
 be better to eliminate the conditional entirely since leddc is always 
 commanded to 0x when the interface is taken down.
 
 local leddc=$(wlc ifname $device leddc)
 [ $leddc = 0x ]  {
  leddc=0x005a000a
 }
 
 what about:
 
 case $leddc in
   '0x'*)
   leddc='0x005a000a'
   ;;
 esac
 
 IMHO the approach from felix does not work:
 root@box:~ echo $((0x))
 65535
 root@box:~ echo $((0x\n))
 -ash: arithmetic syntax error
Your testcase is wrong: \n does not get substituted for a newline here.
If you test with a script that outputs an extra newline, using $(()) works.

- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error

2014-10-18 Thread Nathan Hintz 
Hi Bastian:

 Date: Sat, 18 Oct 2014 22:37:59 +0200
 From: bitt...@bluebottle.com
 To: nlhi...@hotmail.com
 CC: nolt...@gmail.com; openwrt-devel@lists.openwrt.org; ha...@hauke-m.de
 Subject: Re: [OpenWrt-Devel] [PATCH v4] brcm-wl: fix bash comparison error
 
 * Nathan Hintz nlhi...@hotmail.com [18.10.2014 22:24]:
  Using the \n is not correct.  I think the real problem is that the logic 
  is reversed (should be  instead of ||); although it might
  be better to eliminate the conditional entirely since leddc is always 
  commanded to 0x when the interface is taken down.
  
  local leddc=$(wlc ifname $device leddc)
  [ $leddc = 0x ]  {
  leddc=0x005a000a
  }
 
 what about:
 
 case $leddc in
   '0x'*)
   leddc='0x005a000a'
   ;;
 esac
 
 IMHO the approach from felix does not work:
 root@box:~ echo $((0x))
 65535
 root@box:~ echo $((0x\n))
 -ash: arithmetic syntax error
 
 the real would be the fix the output of 'wlc'.

'wlc' is not broken, a simple test case demonstrates it works as expected:

root@e3000-1:~# wlc ifname wl0 leddc
0x005a000a
root@e3000-1:~# wlc ifname wl0 leddc 0x
root@e3000-1:~# wlc ifname wl0 leddc
0x
root@e3000-1:~# leddc=$(wlc ifname wl0 leddc)
root@e3000-1:~# [ $leddc = 0x ]  echo yes
yes
root@e3000-1:~# wlc ifname wl0 leddc 0x005a000a
root@e3000-1:~# wlc ifname wl0 leddc
0x005a000a
root@e3000-1:~# leddc=$(wlc ifname wl0 leddc)
root@e3000-1:~# [ $leddc = 0x ]  echo yes
root@e3000-1:~# 

Nathan

 
 bye bastian
  ___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel