Re: [PATCH] Add WSP_VALUE_TYPE_TEXT support in wsp_decode_application_id()

[Ronald Tessier <ronald.tessier@linux.intel.com>]

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