On 21.05.2012 18:23, Ronald Tessier wrote:
> Hi Jens,
>
> On 05/21/2012 12:54 PM, Jens Rehsack wrote:
>> From: Jens Rehsack<[email protected]>
>>
>> ---
>> 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
> [email protected]
> http://lists.ofono.org/listinfo/ofono
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
