[PATCH] Skip unsolicited CREG/CGREG correctly

[PATCH] Skip unsolicited CREG/CGREG correctly

[Tonny Tzeng]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

This patch skip unsolicited CREG/CGREG correctly.

Signed-off-by: Tonny Tzeng <tonny.tzeng@gmail.com>
---
 drivers/atmodem/atutil.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index b6f0d92..2ca7b44 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -244,8 +244,9 @@ gboolean at_util_parse_reg(GAtResult *result, const
char *prefix,
 			continue;
 
 		/* Some firmware will report bogus lac/ci when unregistered */
+		/* in this case, we should skip it                          */
 		if (s != 1 && s != 5)
-			goto out;
+			continue;
 
 		switch (vendor) {
 		case OFONO_VENDOR_HUAWEI:
-- 
1.7.2.2

Re: [PATCH] Skip unsolicited CREG/CGREG correctly

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

Hi Tonny,

On 12/02/2010 08:23 AM, Tonny Tzeng wrote:
> This patch skip unsolicited CREG/CGREG correctly.
> 
> Signed-off-by: Tonny Tzeng <tonny.tzeng@gmail.com>

Please don't use Signed-off-by

> ---
>  drivers/atmodem/atutil.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
> index b6f0d92..2ca7b44 100644
> --- a/drivers/atmodem/atutil.c
> +++ b/drivers/atmodem/atutil.c
> @@ -244,8 +244,9 @@ gboolean at_util_parse_reg(GAtResult *result, const
> char *prefix,
>  			continue;
>  
>  		/* Some firmware will report bogus lac/ci when unregistered */
> +		/* in this case, we should skip it                          */
>  		if (s != 1 && s != 5)
> -			goto out;
> +			continue;

And this fix is wrong.  what this is doing is skipping the parsing of
the lac/ci values if we're not registered / roaming.  Using continue
here will cause the parser to fail for those cases.

What you probably meant was continuing if the status was not between 1
and 5.  But even that won't really help you if an lac of 1..5 is
encountered ;)

>  
>  		switch (vendor) {
>  		case OFONO_VENDOR_HUAWEI:

Regards,
-Denis

Re: [PATCH] Skip unsolicited CREG/CGREG correctly

[Tonny Tzeng]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

Hi Denis,

Thanks for reviewing.

On Fri, Dec 3, 2010 at 5:22 AM, Denis Kenzior <denkenz@gmail.com> wrote:
>>               /* Some firmware will report bogus lac/ci when unregistered */
>> +             /* in this case, we should skip it                          */
>>               if (s != 1 && s != 5)
>> -                     goto out;
>> +                     continue;
>
> And this fix is wrong.  what this is doing is skipping the parsing of
> the lac/ci values if we're not registered / roaming.  Using continue
> here will cause the parser to fail for those cases.

But if we jump to label 'out', this routine will return the wrong
status code parsed from a unsolicited CREG/CGREG.  For example, in my
case, the real status value goes to the mode argument, and the status
argument got lac value.

Since huawei modems send these strings without quotes, I think we need
different logic to parse these values, for example, we may need a new
function for testing whether there are more codes left?

> What you probably meant was continuing if the status was not between 1
> and 5.  But even that won't really help you if an lac of 1..5 is
> encountered ;)

Yes, you got my point, I will propose another way to avoid this.  Thanks.

Best Regards,
Tonny