Yeah, you committed the proposed change to boxc->boxc_id in revision 15. What I'm asking about is the suggestion and patch I posted here: http://www.kannel.org/pipermail/devel/2010-July/003653.html
2010/7/8 Rene Kluwen <[email protected]>: > It's already in the code. > Current revision is 16. > > == Rene > > -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of > Victor Luchitz > Sent: donderdag 8 juli 2010 7:52 > To: [email protected] > Subject: Re: smppbox code questions > > 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 <[email protected]>: >> 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 >> > > > > -- > Best regards, > Victor Luchitz > > > > -- Best regards, Victor Luchitz
