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;

} 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; }

On 21.05.2012 18:23, Ronald Tessier wrote: > Hi Jens, > > On 05/21/2012 12:54 PM, Jens Rehsack wrote: >> From: Jens Rehsack&lt;sno(a)NetBSD.org&gt; >> >> --- >> 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; >> }