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(a)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