smppbox code questions

Victor Luchitz vluchits at gmail.com
Thu Jul 8 07:52:02 CEST 2010


Any hope this will be reviewed and committed?

I'm also working on a patch that adds TLV support to smppbox but I'd
like to get this one included first.

2010/7/6 Victor Luchitz <vluchits at gmail.com>:
> Yup, it's working fine now. Noticed there's another memleak though:
>
> another octstr_destroy(msgid); call is needed right after the:
> /* we could not find a corresponding dlr; nothing to send */
> line.
>
> I'm also attaching another patch which allows transmission of custom
> error codes in DLR's in the same manner as the message text bit.
>
> 2010/7/6 Rene Kluwen <rene.kluwen at chimit.nl>:
>> I have no way of testing this here. But since either way cannot harm I changed it.
>> Current smppbox revision is now 15.
>> Could you please check out and see if this fixes your problem?
>>
>> == Rene
>>
>> -----Original Message-----
>> From: devel-bounces at kannel.org [mailto:devel-bounces at kannel.org] On Behalf Of Victor Luchitz
>> Sent: dinsdag 6 juli 2010 14:53
>> To: devel at kannel.org
>> Subject: Re: smppbox code questions
>>
>> 1) I think this assumption is incorrect. I have the routing set up
>> this way in bearerbox:
>> group = smsbox-route
>> smsbox-id = vma
>> smsc-id = HTTP
>>
>> So all messages on the 'HTTP' smsc get routed to smppbox. However, the
>> custom HTTP protocol in the above layer does not use dlr_find to route
>> messages to a specific box for two reasons:
>>
>> a) wrong smsc-id is used in the query (bearerbox doesn't know that
>> smppbox overrides the smsc id with system-type) so dlr_find always
>> fails
>> b) dlr_find removes DLR from the table and then subsequently readds
>> it, which is rather stressful on the DB for no sane reason
>>
>> What it does instead is simply setting the sms_type to report_mo,
>> leaving box_id empty as in regular MO messages.
>>
>> 2010/7/6 Rene Kluwen <rene.kluwen at chimit.nl>:
>>> To start with the last thing:
>>>
>>> 2) You are right. It should use the msgid's in the dlr_url from the dlr instance. I changed it.
>>>
>>> About 1): We assume msg->boxc_id and box->boxc_id are the same in this case. Otherwise the message wouldn't have ended up there.
>>>
>>> == Rene
>>>
>>>
>>> -----Original Message-----
>>> From: devel-bounces at kannel.org [mailto:devel-bounces at kannel.org] On Behalf Of Victor Luchitz
>>> Sent: maandag 5 juli 2010 20:36
>>> To: devel at kannel.org
>>> Subject: smppbox code questions
>>>
>>> Hello,
>>>
>>> I have a few questions for you regarding the handling of DLR's by
>>> smppbox, which might also turn out to be bugs.
>>>
>>> 1)
>>> In msg_to_pdu function there's a line which reads:
>>> dlr = dlr_find(msg->sms.boxc_id, msgid, msg->sms.receiver, dlrtype);
>>>
>>> I think it's incorrect because when a DLR is stored by smppbox in
>>> handle_pdu, the boxc_id it uses is that from smpp_logins file
>>> (system_type). That in turn may cause dlr_find to always fail. So in
>>> my opinion the correct dlr_find call is this:
>>> dlr = dlr_find(box->boxc_id, msgid, msg->sms.receiver, dlrtype);
>>>
>>> 2) another thing I find not quite correct is the way smppbox splits
>>> message ids for concatenated DLR's. Basically, what smppbox does is
>>> this:
>>>
>>> parts = octstr_split(msg->sms.dlr_url, octstr_imm(";"));
>>> msgid = gwlist_extract_first(parts);
>>> ...
>>> Then it loops through all elements of the 'parts' list and here is
>>> where the potential problem lies. smppbox assumes that msgid for the
>>> concatenated DLR is always equal to dlr_url which is not always true.
>>> In fact, I think it's never true for concatenated DLR's stored by the
>>> dlr_add call in handle_pdu. Also, for example, the 'msgid' and
>>> 'dlrurls' columns in the storage table can have different maxiumum
>>> lengths, allowing truncation of the msgid. Here's my proposed fix -
>>> add the following bit of code to msg_to_pdu:
>>>
>>> gwlist_destroy(parts, octstr_destroy_item);
>>> parts = octstr_split(dlr->sms.dlr_url, octstr_imm(";"));
>>> gwlist_extract_first(parts);
>>>
>>> right above the following bit:
>>> if (gwlist_len(parts) > 0) {
>>>    while ((msgid2 = gwlist_extract_first(parts)) != NULL) {
>>>
>>>
>>> --
>>> Best regards,
>>>  Victor Luchitz
>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Best regards,
>>  Victor Luchitz
>>
>>
>>
>>
>
>
>
> --
> Best regards,
>  Victor Luchitz
>



-- 
Best regards,
 Victor Luchitz



More information about the devel mailing list