Re: [PATCH] BUG on DLR parsing

Ok, I definitely could do that, the only problem is how will I test  
for it afterwards (not sure if my test binds work with that tlv).

I'll figure out something I guess ;)

Regards,
--
Alejandro Guerrieri
[email protected]



On 30/04/2009, at 14:18, Alexander Malysh wrote:

> Hi,
>
> +1 for this patch :)
>
> As to the original patch, you should first check for  
> network_error_code TLV before going to parse
> message payload. Only of it's not there you should parse payload. So  
> in this form -1 for this patch.
>
> Thanks,
> Alex
>
> P.S. Please integrate both patches together.
>
> Am 30.04.2009 um 12:46 schrieb Alejandro Guerrieri:
>
>> Alex,
>>
>> What about something like this (in addition to my former patch):
>>
>> Index: gw/smsc/smsc_smpp.c
>> ===================================================================
>> --- gw/smsc/smsc_smpp.c	(revision 29)
>> +++ gw/smsc/smsc_smpp.c	(working copy)
>> @@ -1374,11 +1374,16 @@
>>         * we found the delivery report in our storage, so recode the
>>         * message structure.
>>         * The DLR trigger URL is indicated by msg->sms.dlr_url.
>> -         * Add the DLR error code as billing identifier.
>> +         * Add the DLR error code to meta-data.
>>         */
>>        dlrmsg->sms.msgdata = octstr_duplicate(respstr);
>>        dlrmsg->sms.sms_type = report_mo;
>> -        dlrmsg->sms.binfo = octstr_duplicate(err);
>> +        if (err != NULL) {
>> +            if (dlrmsg->sms.meta_data == NULL) {
>> +            	dlrmsg->sms.meta_data = octstr_create("");
>> +            }
>> +        	meta_data_set_value(dlrmsg->sms.meta_data, "smpp",  
>> octstr_imm("dlr_err"), err, 1);
>> +        }
>>    } else {
>>        error(0,"SMPP[%s]: got DLR but could not find message or was  
>> not interested "
>>                "in it id<%s> dst<%s>, type<%d>",
>>
>> Regards,
>> --
>> Alejandro Guerrieri
>> [email protected]
>>
>>
>>
>> On 30/04/2009, at 9:30, Alexander Malysh wrote:
>>
>>> Hi,
>>>
>>> I don't like passing err in binfo field. IMO binfo should be used  
>>> for billing identifier but not for smpp error code.
>>> I would prefer to see patch that drop err from binfo and make use  
>>> of meta-data (group dlr?).
>>>
>>> Thanks,
>>> Alex
>>>
>>> Am 29.04.2009 um 18:10 schrieb Alejandro Guerrieri:
>>>
>>>> This patch fixes a bug when parsing DLR's:
>>>>
>>>> As the code is now, if a DLR having a receipted_message_id, the  
>>>> DLR text is not parsed, so the "err" and "stat" fields are empty.
>>>>
>>>> In particular, the "err" code is being passed on the "binfo"  
>>>> field, so this remained empty if a receipted_message_id is  
>>>> present (because the sscanf code was not executed).
>>>>
>>>> Attached patch fixes that part, so the "err" parameter is present  
>>>> on the binfo field on all cases.
>>>>
>>>> Regards,
>>>> --
>>>> Alejandro Guerrieri
>>>> [email protected]
>>>>
>>>>
>>>> <kannel-smsc-dlr-err.patch>
>>>>
>>>
>>
>