Re: [PATCH] BUG on DLR parsing

Am 30.04.2009 um 15:20 schrieb Alejandro Guerrieri:

> 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 ;)

modify test/drive_smpp.c ;)

>
> 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>
>>>>>
>>>>
>>>
>>
>