Re: [LEDE-DEV] [PATCH] uqmi: ensure CID is a numeric value before proceeding

[Koen Vandeputte]

On 2018-02-19 11:47, Andrey Jr. Melnikov wrote:

Koen Vandeputte  wrote:

The current implementation only checked if uqmi itself executed
correctly which is also the case when the returned value is actually
an error.
Rework this, checking that CID is a numeric value, which can only
be true if uqmi itself also executed correctly.

Why ignore exit status?
if [ $? -ne 0 -o ! "$cid_4" -eq "$cid_4" ]; ...

Because it's pointless to check it here.
Checking the returned value alone also implies uqmi execution went ok.


Also, write comment near this dirty trick, or someone optimize it to
if [ -n "$cid_x" ] ...
Checking for non-empty is wrong, as the return value can be any text 
(like "Error" or "No effect") which would make this test pass wrongly.

CID is only valid when it's an actual pure *numeric* value. (ex: "19")

The interpreter used is basic 'sh' which does not have a direct 
condition check for numeric.
Hours of searching for the cleanest method resulted in this one, being a 
"single line" solution.



Comment was not explicitly included here, as the same trick is also used 
a few lines away [1]

Adding it again and again every few lines just clutters it up


[1] : 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blobdiff;f=package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh;h=bdab5ee5143b5447342cacdfe7f716dd91b76c2b;hp=eba0922e57de8bc413a138de53175a1a713a2c92;hb=3508f8abb492914b6b287b5d60084acb8aff34d2;hpb=3c5471032b9a32a44ee8f5f5b726401fe9eb81e5



Signed-off-by: Koen Vandeputte 
---
  package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh 
b/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
index c3da5ede26b1..46ea134182e3 100755
--- a/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
+++ b/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
@@ -140,11 +140,11 @@ proto_qmi_setup() {
  
 [ "$pdptype" = "ip" -o "$pdptype" = "ipv4v6" ] && {

 cid_4=$(uqmi -s -d "$device" --get-client-id wds)
-   [ $? -ne 0 ] && {
+   if ! [ "$cid_4" -eq "$cid_4" ] 2> /dev/null; then
 echo "Unable to obtain client ID"
 proto_notify_error "$interface" NO_CID
 return 1
-   }
+   fi
  
 uqmi -s -d "$device" --set-client-id wds,"$cid_4" --set-ip-family ipv4 > /dev/null
  
@@ -177,11 +177,11 @@ proto_qmi_setup() {
  
 [ "$pdptype" = "ipv6" -o "$pdptype" = "ipv4v6" ] && {

 cid_6=$(uqmi -s -d "$device" --get-client-id wds)
-   [ $? -ne 0 ] && {
+   if ! [ "$cid_6" -eq "$cid_6" ] 2> /dev/null; then
 echo "Unable to obtain client ID"
 proto_notify_error "$interface" NO_CID
 return 1
-   }
+   fi
  
 uqmi -s -d "$device" --set-client-id wds,"$cid_6" --set-ip-family ipv6 > /dev/null
  
--

2.7.4


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH] uqmi: ensure CID is a numeric value before proceeding

[Andrey Jr. Melnikov]

Koen Vandeputte  wrote:
> The current implementation only checked if uqmi itself executed
> correctly which is also the case when the returned value is actually
> an error.

> Rework this, checking that CID is a numeric value, which can only
> be true if uqmi itself also executed correctly.

Why ignore exit status?
if [ $? -ne 0 -o ! "$cid_4" -eq "$cid_4" ]; ...

Also, write comment near this dirty trick, or someone optimize it to
if [ -n "$cid_x" ] ... 

> Signed-off-by: Koen Vandeputte 
> ---
>  package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

> diff --git a/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh 
> b/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
> index c3da5ede26b1..46ea134182e3 100755
> --- a/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
> +++ b/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
> @@ -140,11 +140,11 @@ proto_qmi_setup() {
>  
> [ "$pdptype" = "ip" -o "$pdptype" = "ipv4v6" ] && {
> cid_4=$(uqmi -s -d "$device" --get-client-id wds)
> -   [ $? -ne 0 ] && {
> +   if ! [ "$cid_4" -eq "$cid_4" ] 2> /dev/null; then
> echo "Unable to obtain client ID"
> proto_notify_error "$interface" NO_CID
> return 1
> -   }
> +   fi
>  
> uqmi -s -d "$device" --set-client-id wds,"$cid_4" 
> --set-ip-family ipv4 > /dev/null
>  
> @@ -177,11 +177,11 @@ proto_qmi_setup() {
>  
> [ "$pdptype" = "ipv6" -o "$pdptype" = "ipv4v6" ] && {
> cid_6=$(uqmi -s -d "$device" --get-client-id wds)
> -   [ $? -ne 0 ] && {
> +   if ! [ "$cid_6" -eq "$cid_6" ] 2> /dev/null; then
> echo "Unable to obtain client ID"
> proto_notify_error "$interface" NO_CID
> return 1
> -   }
> +   fi
>  
> uqmi -s -d "$device" --set-client-id wds,"$cid_6" 
> --set-ip-family ipv6 > /dev/null
>  
> -- 
> 2.7.4


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

[LEDE-DEV] [PATCH] uqmi: ensure CID is a numeric value before proceeding

[Koen Vandeputte]

The current implementation only checked if uqmi itself executed
correctly which is also the case when the returned value is actually
an error.

Rework this, checking that CID is a numeric value, which can only
be true if uqmi itself also executed correctly.

Signed-off-by: Koen Vandeputte 
---
 package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh 
b/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
index c3da5ede26b1..46ea134182e3 100755
--- a/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
+++ b/package/network/utils/uqmi/files/lib/netifd/proto/qmi.sh
@@ -140,11 +140,11 @@ proto_qmi_setup() {
 
[ "$pdptype" = "ip" -o "$pdptype" = "ipv4v6" ] && {
cid_4=$(uqmi -s -d "$device" --get-client-id wds)
-   [ $? -ne 0 ] && {
+   if ! [ "$cid_4" -eq "$cid_4" ] 2> /dev/null; then
echo "Unable to obtain client ID"
proto_notify_error "$interface" NO_CID
return 1
-   }
+   fi
 
uqmi -s -d "$device" --set-client-id wds,"$cid_4" 
--set-ip-family ipv4 > /dev/null
 
@@ -177,11 +177,11 @@ proto_qmi_setup() {
 
[ "$pdptype" = "ipv6" -o "$pdptype" = "ipv4v6" ] && {
cid_6=$(uqmi -s -d "$device" --get-client-id wds)
-   [ $? -ne 0 ] && {
+   if ! [ "$cid_6" -eq "$cid_6" ] 2> /dev/null; then
echo "Unable to obtain client ID"
proto_notify_error "$interface" NO_CID
return 1
-   }
+   fi
 
uqmi -s -d "$device" --set-client-id wds,"$cid_6" 
--set-ip-family ipv6 > /dev/null
 
-- 
2.7.4


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev