Re: [PATCH 1/3] Fix CTLV three-byte tag parsing.

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]

Hi Andrew,

> ---
>  src/simutil.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/simutil.c b/src/simutil.c
> index 9fb111f..822938c 100644
> --- a/src/simutil.c
> +++ b/src/simutil.c
> @@ -210,7 +210,7 @@ void comprehension_tlv_iter_init(struct
>  comprehension_tlv_iter *iter, iter->data = 0;
>  }
> 
> -/* Comprehension TLVs defined in Section 7 of ETSI TS 102.220 */
> +/* Comprehension TLVs defined in Section 7 of ETSI TS 101.220 */
>  gboolean comprehension_tlv_iter_next(struct comprehension_tlv_iter *iter)
>  {
>  	const unsigned char *pdu = iter->pdu + iter->pos;
> @@ -230,18 +230,18 @@ gboolean comprehension_tlv_iter_next(struct
>  comprehension_tlv_iter *iter) return FALSE;
> 
>  	/*
> -	 * ETSI TS 102.220, Section 7.1.1.2
> +	 * ETSI TS 101.220, Section 7.1.1.2
>  	 *
>  	 * If byte 1 of the tag is equal to 0x7F, then the tag is encoded
> -	 * on the following two bytes, with bit 8 of the 2nd byte of the tag
> -	 * being the CR flag.
> +	 * on the following two bytes, with bit 8 of the 1st byte of the tag
> +	 * as the most significant bit.

Please keep the comment about the CR flag, I think that is useful. 

>  	 */
> -	if (tag == 0x7F) {
> +	if (tag == 0x7F && !cr) {

This isn't actually necessary.  However, the statement:

        if (tag == 0x00 || tag == 0xFF || tag == 0x80)
                return FALSE;

Actually needs to be moved up and tag changed to *pdu.

>  		if ((pdu + 2) > end)
>  			return FALSE;
> 
>  		cr = bit_field(pdu[0], 7, 1);
> -		tag = ((pdu[0] & 0x7f) << 7) | pdu[1];
> +		tag = ((pdu[0] & 0x7f) << 8) | pdu[1];
> 
>  		if (tag < 0x0001 || tag > 0x7fff)
>  			return FALSE;
> 

Can you resubmit the patch with these changes?

Thanks,
-Denis