[PATCH 12/20] stkutil: Add the Call Control envelope builder

[Andrzej Zaborowski]

Hi,

On 10 June 2010 00:57, Denis Kenzior <denkenz at gmail.com> wrote:
>> ---
>>  src/stkutil.c |  178
>>  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> src/stkutil.h |
>>   31 ++++++++++
>>  2 files changed, 207 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/stkutil.c b/src/stkutil.c
>> index 83ff5c7..b9a152a 100644
>> --- a/src/stkutil.c
>> +++ b/src/stkutil.c
>> @@ -319,8 +319,6 @@ static gboolean parse_dataobj_subaddress(struct
>>  comprehension_tlv_iter *iter, unsigned int len;
>>
>>       len = comprehension_tlv_iter_get_length(iter);
>> -     if (len < 1)
>> -             return FALSE;
>
> This seems wrong.  Can you point me to the test that uses empty subaddresses?

I removed this from the new version of the patch.  Empty subaddress
("null object") is only used in the response to the Call Control
ENVELOPE (second half of section 7.3.1.6).

>
>> +/* Used both in the ENVELOPE message to UICC and response from UICC */
>> +struct stk_envelope_call_control {
>> +     /* Exactly one of the following 5 fields must be present */
>
> Why are we not using a union then?

We don't know which one of 5 is valid if it's a union.  We could add a
field to indicate that but I guess it's simpler the way it is.

>
>> +     struct stk_address address;
>> +     struct stk_address ss_string;
>> +     struct stk_ussd_string {
>> +             unsigned char dcs;
>> +             const unsigned char *string;
>
> Please don't do that.  These objects will be passed around like candy and I
> don't want to deal with dangling pointers.
>
>> +             int len;
>> +     } ussd_string;
>
> The ussd string is limited to 160 bytes according to 23.038.  So breaking this
> out into a proper stk datatype like:
> struct stk_ussd_string {
>        unsigned char dcs;
>        unsigned char ussd[160];
>        int len;
> };
> would be better

Ok.  I'll resend these four patches and add the remaining ENVELOPEs.

Regards