All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
@ 2012-05-16 14:03 Jens Rehsack
  2012-05-16 15:45 ` Ronald Tessier
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Rehsack @ 2012-05-16 14:03 UTC (permalink / raw)
  To: ofono

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

From: Jens Rehsack <sno@NetBSD.org>

---
 src/wsputil.c |    6 ++++++
 1 Datei geändert, 6 Zeilen hinzugefügt(+)

diff --git a/src/wsputil.c b/src/wsputil.c
index 1b2b2b7..c4c9d62 100644
--- a/src/wsputil.c
+++ b/src/wsputil.c
@@ -496,6 +496,12 @@ gboolean wsp_decode_application_id(struct wsp_header_iter *iter,
 	 */
 	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
 		val = *pdu_val & 0x7f;
+	} else if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
+		val_len = wsp_header_iter_get_val_len(iter);
+		val = -1;
+
+		if (out_value)
+			*out_value = pdu_val;
 	} else {
 		val_len = wsp_header_iter_get_val_len(iter);
 
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-16 14:03 Jens Rehsack
@ 2012-05-16 15:45 ` Ronald Tessier
  2012-05-16 16:18   ` Jens Rehsack
  0 siblings, 1 reply; 10+ messages in thread
From: Ronald Tessier @ 2012-05-16 15:45 UTC (permalink / raw)
  To: ofono

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

Hi Jens,

On 05/16/2012 04:03 PM, Jens Rehsack wrote:
> From: Jens Rehsack<sno@NetBSD.org>
>
> ---
>   src/wsputil.c |    6 ++++++
>   1 Datei geändert, 6 Zeilen hinzugefügt(+)
>
> diff --git a/src/wsputil.c b/src/wsputil.c
> index 1b2b2b7..c4c9d62 100644
> --- a/src/wsputil.c
> +++ b/src/wsputil.c
> @@ -496,6 +496,12 @@ gboolean wsp_decode_application_id(struct wsp_header_iter *iter,
>   	 */
>   	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
>   		val = *pdu_val&  0x7f;
> +	} else if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
> +		val_len = wsp_header_iter_get_val_len(iter);
> +		val = -1;
> +
> +		if (out_value)
> +			*out_value = pdu_val;
>   	} else {
>   		val_len = wsp_header_iter_get_val_len(iter);
>

Unless I miss something, if the application_id is given as TEXT (in the 
pdu) the *out_value will always be NULL (it will be set to the result of 
get_text_entry()).

I think you don't need to set val_len and val, you can just return TRUE 
after doing *out_value = pdu_val.

Best regards,

Ronald

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-16 15:45 ` Ronald Tessier
@ 2012-05-16 16:18   ` Jens Rehsack
  2012-05-18 12:59     ` Ronald Tessier
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Rehsack @ 2012-05-16 16:18 UTC (permalink / raw)
  To: ofono

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

On 16.05.2012 17:45, Ronald Tessier wrote:
> Hi Jens,

Hi Ronald,

> On 05/16/2012 04:03 PM, Jens Rehsack wrote:
>> From: Jens Rehsack<sno@NetBSD.org>
>>
>> ---
>>   src/wsputil.c |    6 ++++++
>>   1 Datei geändert, 6 Zeilen hinzugefügt(+)
>>
>> diff --git a/src/wsputil.c b/src/wsputil.c
>> index 1b2b2b7..c4c9d62 100644
>> --- a/src/wsputil.c
>> +++ b/src/wsputil.c
>> @@ -496,6 +496,12 @@ gboolean wsp_decode_application_id(struct
>> wsp_header_iter *iter,
>>        */
>>       if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
>>           val = *pdu_val&  0x7f;
>> +    } else if (wsp_header_iter_get_val_type(iter) ==
>> WSP_VALUE_TYPE_TEXT) {
>> +        val_len = wsp_header_iter_get_val_len(iter);
>> +        val = -1;
>> +
>> +        if (out_value)
>> +            *out_value = pdu_val;
>>       } else {
>>           val_len = wsp_header_iter_get_val_len(iter);
>>
> 
> Unless I miss something, if the application_id is given as TEXT (in the
> pdu) the *out_value will always be NULL (it will be set to the result of
> get_text_entry()).
> 
> I think you don't need to set val_len and val, you can just return TRUE
> after doing *out_value = pdu_val.

Probably. Do you want me to act on this and submit a new version (not
before Monday - commuting now) or will you fix it on your own?

Best regards,
Jens

> Best regards,
> 
> Ronald
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-16 16:18   ` Jens Rehsack
@ 2012-05-18 12:59     ` Ronald Tessier
  0 siblings, 0 replies; 10+ messages in thread
From: Ronald Tessier @ 2012-05-18 12:59 UTC (permalink / raw)
  To: ofono

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

Hi Jens,

On 05/16/2012 06:18 PM, Jens Rehsack wrote:
> On 16.05.2012 17:45, Ronald Tessier wrote:
>> Hi Jens,
>
> Hi Ronald,
>
>> On 05/16/2012 04:03 PM, Jens Rehsack wrote:
>>> From: Jens Rehsack<sno@NetBSD.org>
>>>
>>> ---
>>>    src/wsputil.c |    6 ++++++
>>>    1 Datei geändert, 6 Zeilen hinzugefügt(+)
>>>
>>> diff --git a/src/wsputil.c b/src/wsputil.c
>>> index 1b2b2b7..c4c9d62 100644
>>> --- a/src/wsputil.c
>>> +++ b/src/wsputil.c
>>> @@ -496,6 +496,12 @@ gboolean wsp_decode_application_id(struct
>>> wsp_header_iter *iter,
>>>         */
>>>        if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
>>>            val = *pdu_val&   0x7f;
>>> +    } else if (wsp_header_iter_get_val_type(iter) ==
>>> WSP_VALUE_TYPE_TEXT) {
>>> +        val_len = wsp_header_iter_get_val_len(iter);
>>> +        val = -1;
>>> +
>>> +        if (out_value)
>>> +            *out_value = pdu_val;
>>>        } else {
>>>            val_len = wsp_header_iter_get_val_len(iter);
>>>
>>
>> Unless I miss something, if the application_id is given as TEXT (in the
>> pdu) the *out_value will always be NULL (it will be set to the result of
>> get_text_entry()).
>>
>> I think you don't need to set val_len and val, you can just return TRUE
>> after doing *out_value = pdu_val.
>
> Probably. Do you want me to act on this and submit a new version (not
> before Monday - commuting now) or will you fix it on your own?
>

Indeed it can easily wait until Monday ...
Please, submit a new version of your patch to be reviewed by a maintainer.

Best regards,

Ronald

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
@ 2012-05-21 10:54 Jens Rehsack
  2012-05-21 16:23 ` Ronald Tessier
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Rehsack @ 2012-05-21 10:54 UTC (permalink / raw)
  To: ofono

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

From: Jens Rehsack <sno@NetBSD.org>

---
 src/wsputil.c |   43 ++++++++++++++++++++++++++-----------------
 1 Datei geändert, 26 Zeilen hinzugefügt(+), 17 Zeilen entfernt(-)

diff --git a/src/wsputil.c b/src/wsputil.c
index 1b2b2b7..5611ed2 100644
--- a/src/wsputil.c
+++ b/src/wsputil.c
@@ -486,28 +486,37 @@ gboolean wsp_decode_application_id(struct wsp_header_iter *iter,
 							const void **out_value)
 {
 	const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
-	unsigned int val;
-	unsigned int val_len;
-	unsigned int i;
 
-	/*
-	 * Well-known field values MUST be encoded using the
-	 * compact binary formats
-	 */
-	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
-		val = *pdu_val & 0x7f;
+	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
+		wsp_header_iter_get_val_len(iter);
+
+		if (out_value)
+			*out_value = pdu_val;
 	} else {
-		val_len = wsp_header_iter_get_val_len(iter);
+		unsigned int val;
 
-		if (val_len > 2)
-			return FALSE;
+		/*
+		 * Well-known field values MUST be encoded using the
+		 * compact binary formats
+		 */
+		if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
+			val = *pdu_val & 0x7f;
+		} else  {
+			unsigned int val_len;
+			unsigned int i;
 
-		for (i = 0, val = 0; i < val_len && i < sizeof(val); i++)
-			val = (val << 8) | pdu_val[i];
-	}
+			val_len = wsp_header_iter_get_val_len(iter);
 
-	if (out_value)
-		*out_value = get_text_entry(val, app_id);
+			if (val_len > 2)
+				return FALSE;
+
+			for (i = 0, val = 0; i < val_len && i < sizeof(val); i++)
+				val = (val << 8) | pdu_val[i];
+		}
+
+		if (out_value)
+			*out_value = get_text_entry(val, app_id);
+	}
 
 	return TRUE;
 }
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-21 10:54 Jens Rehsack
@ 2012-05-21 16:23 ` Ronald Tessier
  2012-05-22  8:09   ` Jens Rehsack
  0 siblings, 1 reply; 10+ messages in thread
From: Ronald Tessier @ 2012-05-21 16:23 UTC (permalink / raw)
  To: ofono

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

Hi Jens,

On 05/21/2012 12:54 PM, Jens Rehsack wrote:
> From: Jens Rehsack<sno@NetBSD.org>
>
> ---
>   src/wsputil.c |   43 ++++++++++++++++++++++++++-----------------
>   1 Datei geändert, 26 Zeilen hinzugefügt(+), 17 Zeilen entfernt(-)
>
> diff --git a/src/wsputil.c b/src/wsputil.c
> index 1b2b2b7..5611ed2 100644
> --- a/src/wsputil.c
> +++ b/src/wsputil.c
> @@ -486,28 +486,37 @@ gboolean wsp_decode_application_id(struct wsp_header_iter *iter,
>   							const void **out_value)
>   {
>   	const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
> -	unsigned int val;
> -	unsigned int val_len;
> -	unsigned int i;
>
> -	/*
> -	 * Well-known field values MUST be encoded using the
> -	 * compact binary formats
> -	 */
> -	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
> -		val = *pdu_val&  0x7f;
> +	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
> +		wsp_header_iter_get_val_len(iter);
> +
> +		if (out_value)
> +			*out_value = pdu_val;

Why not just returning if the type is TEXT ?
Something like :

	const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
	unsigned int val;
	unsigned int val_len;
	unsigned int i;

+	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
+		if (out_value)
+			*out_value = pdu_val;
+
+		return TRUE;
+	}
+
	/*
	 * Well-known field values MUST be encoded using the
	 * compact binary formats
	 */
	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {


I think it does the same thing but more readable (you don't break the 80 
col. limit).

Best regards,

Ronald


>   	} else {
> -		val_len = wsp_header_iter_get_val_len(iter);
> +		unsigned int val;
>
> -		if (val_len>  2)
> -			return FALSE;
> +		/*
> +		 * Well-known field values MUST be encoded using the
> +		 * compact binary formats
> +		 */
> +		if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
> +			val = *pdu_val&  0x7f;
> +		} else  {
> +			unsigned int val_len;
> +			unsigned int i;
>
> -		for (i = 0, val = 0; i<  val_len&&  i<  sizeof(val); i++)
> -			val = (val<<  8) | pdu_val[i];
> -	}
> +			val_len = wsp_header_iter_get_val_len(iter);
>
> -	if (out_value)
> -		*out_value = get_text_entry(val, app_id);
> +			if (val_len>  2)
> +				return FALSE;
> +
> +			for (i = 0, val = 0; i<  val_len&&  i<  sizeof(val); i++)
> +				val = (val<<  8) | pdu_val[i];
> +		}
> +
> +		if (out_value)
> +			*out_value = get_text_entry(val, app_id);
> +	}
>
>   	return TRUE;
>   }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-21 16:23 ` Ronald Tessier
@ 2012-05-22  8:09   ` Jens Rehsack
  2012-05-30  7:11     ` Jens Rehsack
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Rehsack @ 2012-05-22  8:09 UTC (permalink / raw)
  To: ofono

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

On 21.05.2012 18:23, Ronald Tessier wrote:
> Hi Jens,
> 
> On 05/21/2012 12:54 PM, Jens Rehsack wrote:
>> From: Jens Rehsack<sno@NetBSD.org>
>>
>> ---
>>   src/wsputil.c |   43 ++++++++++++++++++++++++++-----------------
>>   1 Datei geändert, 26 Zeilen hinzugefügt(+), 17 Zeilen entfernt(-)
>>
>> diff --git a/src/wsputil.c b/src/wsputil.c
>> index 1b2b2b7..5611ed2 100644
>> --- a/src/wsputil.c
>> +++ b/src/wsputil.c
>> @@ -486,28 +486,37 @@ gboolean wsp_decode_application_id(struct
>> wsp_header_iter *iter,
>>                               const void **out_value)
>>   {
>>       const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
>> -    unsigned int val;
>> -    unsigned int val_len;
>> -    unsigned int i;
>>
>> -    /*
>> -     * Well-known field values MUST be encoded using the
>> -     * compact binary formats
>> -     */
>> -    if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
>> -        val = *pdu_val&  0x7f;
>> +    if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
>> +        wsp_header_iter_get_val_len(iter);
>> +
>> +        if (out_value)
>> +            *out_value = pdu_val;
> 
> Why not just returning if the type is TEXT ?
> Something like :
> 
>     const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
>     unsigned int val;
>     unsigned int val_len;
>     unsigned int i;
> 
> +    if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
> +        if (out_value)
> +            *out_value = pdu_val;
> +
> +        return TRUE;
> +    }
> +
>     /*
>      * Well-known field values MUST be encoded using the
>      * compact binary formats
>      */
>     if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
> 

I do not like several returns in a longer subroutine. The way
you format code makes code longer - fair enough, but for me to
long to introduce early return statements.

> I think it does the same thing but more readable (you don't break the 80
> col. limit).

I'm fine with your way to do it, too. But I wouldn't want my name being
assigned with it (because of the early return ...). I strongly
distinguish between readable and understandable, and I prefer the
understandable way.

How about:

diff --git a/src/wsputil.c b/src/wsputil.c
index 1b2b2b7..385acaa 100644
--- a/src/wsputil.c
+++ b/src/wsputil.c
@@ -490,13 +490,26 @@ gboolean wsp_decode_application_id(struct
wsp_header_iter *iter,
        unsigned int val_len;
        unsigned int i;

+       switch (wsp_header_iter_get_val_type(iter)) {
+       case WSP_VALUE_TYPE_TEXT:
+               if (out_value)
+                       *out_value = pdu_val;
+
+               break;
+
        /*
         * Well-known field values MUST be encoded using the
         * compact binary formats
         */
-       if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
+       case WSP_VALUE_TYPE_SHORT:
                val = *pdu_val & 0x7f;
-       } else {
+
+               if (out_value)
+                       *out_value = get_text_entry(val, app_id);
+
+               break;
+
+       default:
                val_len = wsp_header_iter_get_val_len(iter);

                if (val_len > 2)
@@ -504,10 +517,13 @@ gboolean wsp_decode_application_id(struct
wsp_header_iter *iter,

                for (i = 0, val = 0; i < val_len && i < sizeof(val); i++)
                        val = (val << 8) | pdu_val[i];
+
+               if (out_value)
+                       *out_value = get_text_entry(val, app_id);
+
+               break;
        }

-       if (out_value)
-               *out_value = get_text_entry(val, app_id);

        return TRUE;
 }


> Best regards,
> 
> Ronald

Best regards,
Jens

>>       } else {
>> -        val_len = wsp_header_iter_get_val_len(iter);
>> +        unsigned int val;
>>
>> -        if (val_len>  2)
>> -            return FALSE;
>> +        /*
>> +         * Well-known field values MUST be encoded using the
>> +         * compact binary formats
>> +         */
>> +        if (wsp_header_iter_get_val_type(iter) ==
>> WSP_VALUE_TYPE_SHORT) {
>> +            val = *pdu_val&  0x7f;
>> +        } else  {
>> +            unsigned int val_len;
>> +            unsigned int i;
>>
>> -        for (i = 0, val = 0; i<  val_len&&  i<  sizeof(val); i++)
>> -            val = (val<<  8) | pdu_val[i];
>> -    }
>> +            val_len = wsp_header_iter_get_val_len(iter);
>>
>> -    if (out_value)
>> -        *out_value = get_text_entry(val, app_id);
>> +            if (val_len>  2)
>> +                return FALSE;
>> +
>> +            for (i = 0, val = 0; i<  val_len&&  i<  sizeof(val); i++)
>> +                val = (val<<  8) | pdu_val[i];
>> +        }
>> +
>> +        if (out_value)
>> +            *out_value = get_text_entry(val, app_id);
>> +    }
>>
>>       return TRUE;
>>   }
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-22  8:09   ` Jens Rehsack
@ 2012-05-30  7:11     ` Jens Rehsack
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Rehsack @ 2012-05-30  7:11 UTC (permalink / raw)
  To: ofono

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

On 22.05.2012 10:09, Jens Rehsack wrote:
> On 21.05.2012 18:23, Ronald Tessier wrote:
>> Hi Jens,
>>
>> On 05/21/2012 12:54 PM, Jens Rehsack wrote:
>>> From: Jens Rehsack<sno@NetBSD.org>
>>>
>>> ---
>>>   src/wsputil.c |   43 ++++++++++++++++++++++++++-----------------
>>>   1 Datei geändert, 26 Zeilen hinzugefügt(+), 17 Zeilen entfernt(-)
>>>
>>> diff --git a/src/wsputil.c b/src/wsputil.c
>>> index 1b2b2b7..5611ed2 100644
>>> --- a/src/wsputil.c
>>> +++ b/src/wsputil.c
>>> @@ -486,28 +486,37 @@ gboolean wsp_decode_application_id(struct
>>> wsp_header_iter *iter,
>>>                               const void **out_value)
>>>   {
>>>       const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
>>> -    unsigned int val;
>>> -    unsigned int val_len;
>>> -    unsigned int i;
>>>
>>> -    /*
>>> -     * Well-known field values MUST be encoded using the
>>> -     * compact binary formats
>>> -     */
>>> -    if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
>>> -        val = *pdu_val&  0x7f;
>>> +    if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
>>> +        wsp_header_iter_get_val_len(iter);
>>> +
>>> +        if (out_value)
>>> +            *out_value = pdu_val;
>>
>> Why not just returning if the type is TEXT ?
>> Something like :
>>
>>     const unsigned char *pdu_val = wsp_header_iter_get_val(iter);
>>     unsigned int val;
>>     unsigned int val_len;
>>     unsigned int i;
>>
>> +    if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_TEXT) {
>> +        if (out_value)
>> +            *out_value = pdu_val;
>> +
>> +        return TRUE;
>> +    }
>> +
>>     /*
>>      * Well-known field values MUST be encoded using the
>>      * compact binary formats
>>      */
>>     if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
>>
> 
> I do not like several returns in a longer subroutine. The way
> you format code makes code longer - fair enough, but for me to
> long to introduce early return statements.
> 
>> I think it does the same thing but more readable (you don't break the 80
>> col. limit).
> 
> I'm fine with your way to do it, too. But I wouldn't want my name being
> assigned with it (because of the early return ...). I strongly
> distinguish between readable and understandable, and I prefer the
> understandable way.
> 
> How about:
> 
> diff --git a/src/wsputil.c b/src/wsputil.c
> index 1b2b2b7..385acaa 100644
> --- a/src/wsputil.c
> +++ b/src/wsputil.c
> @@ -490,13 +490,26 @@ gboolean wsp_decode_application_id(struct
> wsp_header_iter *iter,
>         unsigned int val_len;
>         unsigned int i;
> 
> +       switch (wsp_header_iter_get_val_type(iter)) {
> +       case WSP_VALUE_TYPE_TEXT:
> +               if (out_value)
> +                       *out_value = pdu_val;
> +
> +               break;
> +
>         /*
>          * Well-known field values MUST be encoded using the
>          * compact binary formats
>          */
> -       if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
> +       case WSP_VALUE_TYPE_SHORT:
>                 val = *pdu_val & 0x7f;
> -       } else {
> +
> +               if (out_value)
> +                       *out_value = get_text_entry(val, app_id);
> +
> +               break;
> +
> +       default:
>                 val_len = wsp_header_iter_get_val_len(iter);
> 
>                 if (val_len > 2)
> @@ -504,10 +517,13 @@ gboolean wsp_decode_application_id(struct
> wsp_header_iter *iter,
> 
>                 for (i = 0, val = 0; i < val_len && i < sizeof(val); i++)
>                         val = (val << 8) | pdu_val[i];
> +
> +               if (out_value)
> +                       *out_value = get_text_entry(val, app_id);
> +
> +               break;
>         }
> 
> -       if (out_value)
> -               *out_value = get_text_entry(val, app_id);
> 
>         return TRUE;
>  }
> 
> 
>> Best regards,
>>
>> Ronald
> 
> Best regards,
> Jens
> 
>>>       } else {
>>> -        val_len = wsp_header_iter_get_val_len(iter);
>>> +        unsigned int val;
>>>
>>> -        if (val_len>  2)
>>> -            return FALSE;
>>> +        /*
>>> +         * Well-known field values MUST be encoded using the
>>> +         * compact binary formats
>>> +         */
>>> +        if (wsp_header_iter_get_val_type(iter) ==
>>> WSP_VALUE_TYPE_SHORT) {
>>> +            val = *pdu_val&  0x7f;
>>> +        } else  {
>>> +            unsigned int val_len;
>>> +            unsigned int i;
>>>
>>> -        for (i = 0, val = 0; i<  val_len&&  i<  sizeof(val); i++)
>>> -            val = (val<<  8) | pdu_val[i];
>>> -    }
>>> +            val_len = wsp_header_iter_get_val_len(iter);
>>>
>>> -    if (out_value)
>>> -        *out_value = get_text_entry(val, app_id);
>>> +            if (val_len>  2)
>>> +                return FALSE;
>>> +
>>> +            for (i = 0, val = 0; i<  val_len&&  i<  sizeof(val); i++)
>>> +                val = (val<<  8) | pdu_val[i];
>>> +        }
>>> +
>>> +        if (out_value)
>>> +            *out_value = get_text_entry(val, app_id);
>>> +    }
>>>
>>>       return TRUE;
>>>   }

What's the state?

/Jens


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
@ 2012-05-30  8:59 Jens Rehsack
  2012-05-30 13:49 ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Rehsack @ 2012-05-30  8:59 UTC (permalink / raw)
  To: ofono

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

From: Jens Rehsack <sno@NetBSD.org>

---
 src/wsputil.c |   24 ++++++++++++++++++++----
 1 Datei geändert, 20 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-)

diff --git a/src/wsputil.c b/src/wsputil.c
index 1b2b2b7..ece58e6 100644
--- a/src/wsputil.c
+++ b/src/wsputil.c
@@ -490,13 +490,26 @@ gboolean wsp_decode_application_id(struct wsp_header_iter *iter,
 	unsigned int val_len;
 	unsigned int i;
 
+	switch (wsp_header_iter_get_val_type(iter)) {
+	case WSP_VALUE_TYPE_TEXT:
+		if (out_value)
+			*out_value = pdu_val;
+
+		break;
+
 	/*
 	 * Well-known field values MUST be encoded using the
 	 * compact binary formats
 	 */
-	if (wsp_header_iter_get_val_type(iter) == WSP_VALUE_TYPE_SHORT) {
+	case WSP_VALUE_TYPE_SHORT:
 		val = *pdu_val & 0x7f;
-	} else {
+
+		if (out_value)
+			*out_value = get_text_entry(val, app_id);
+
+		break;
+
+	case WSP_VALUE_TYPE_LONG:
 		val_len = wsp_header_iter_get_val_len(iter);
 
 		if (val_len > 2)
@@ -504,10 +517,13 @@ gboolean wsp_decode_application_id(struct wsp_header_iter *iter,
 
 		for (i = 0, val = 0; i < val_len && i < sizeof(val); i++)
 			val = (val << 8) | pdu_val[i];
+
+		if (out_value)
+			*out_value = get_text_entry(val, app_id);
+
+		break;
 	}
 
-	if (out_value)
-		*out_value = get_text_entry(val, app_id);
 
 	return TRUE;
 }
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()
  2012-05-30  8:59 [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id() Jens Rehsack
@ 2012-05-30 13:49 ` Denis Kenzior
  0 siblings, 0 replies; 10+ messages in thread
From: Denis Kenzior @ 2012-05-30 13:49 UTC (permalink / raw)
  To: ofono

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

Hi Jens,

On 05/30/2012 03:59 AM, Jens Rehsack wrote:
> From: Jens Rehsack<sno@NetBSD.org>
>
> ---
>   src/wsputil.c |   24 ++++++++++++++++++++----
>   1 Datei geändert, 20 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-)
>

Patch has been applied with a slightly reworded commit message and a 
whitespace fix.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-05-30 13:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-30  8:59 [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id() Jens Rehsack
2012-05-30 13:49 ` Denis Kenzior
  -- strict thread matches above, loose matches on Subject: below --
2012-05-21 10:54 Jens Rehsack
2012-05-21 16:23 ` Ronald Tessier
2012-05-22  8:09   ` Jens Rehsack
2012-05-30  7:11     ` Jens Rehsack
2012-05-16 14:03 Jens Rehsack
2012-05-16 15:45 ` Ronald Tessier
2012-05-16 16:18   ` Jens Rehsack
2012-05-18 12:59     ` Ronald Tessier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.