* [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* [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-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
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.