[PATCH 01/11] stkutil: Add SMS-PP Data Download envelope builder

[Andrzej Zaborowski]

Hi Denis,

On 2 June 2010 01:50, Denis Kenzior <denkenz at gmail.com> wrote:
> Hi Andrew,
>
>> Note that the sms_tpdu member could be of type struct sms_deliver, but the
>> users may more often have access to the raw tpdu so there's no point
>> decoding and reencoding it.
>
> Overall patch looks fine, but using a raw pdu is not preferable.  Couple of
> reasons:
>
> - When SMS is delivered, it is delivered in tpdu form, e.g. sc_address +
> deliver pdu.  To obtain the sc_address, we need to decode the deliver tpdu
> anyway.
> - Before we know this is an SMS-PP download, we must check the dcs / pid.  In
> order to check those, we must again decode the tpdu.
> - SMS sc_address can actually be non-numeric.  In that case the SMS-PP
> download should simply be dropped.

This means we have to decode the PDU, but re-encoding it is still an
overhead having access to the TPDU.

> - Consistency with Send SMS proactive command.

Ok, makes sense.

>
>> +
>> +/* Returns TRUE on success */
>> +ofono_bool_t stk_pdu_from_envelope(const struct stk_envelope *envelope,
>> +                                     unsigned char *pdu, unsigned int size,
>> +                                     unsigned char **out_pdu,
>> +                                     unsigned int *out_size);
>>
>
> This part just looks ugly.  Can't we hide the details of char buf[512]
> somewhere inside stk_pdu_from_envelope?

By that do you mean using a static buffer?  I'll send a patch for
that.  I prefer a static buffer for the PDUs but thought you had
argued against it :)

Regards