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 <[email protected]>:
> 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: [email protected] [mailto:[email protected]] On Behalf Of 
> Victor Luchitz
> Sent: dinsdag 6 juli 2010 14:53
> To: [email protected]
> 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 <[email protected]>:
>> 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: [email protected] [mailto:[email protected]] On Behalf 
>> Of Victor Luchitz
>> Sent: maandag 5 juli 2010 20:36
>> To: [email protected]
>> 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

Attachment: err_patch.diff
Description: Binary data

Reply via email to