Re: [PATCH] BUG on DLR parsing

Ooook, didn't know about that one :)

Thanks!
--
Alejandro Guerrieri
[email protected]



On 30/04/2009, at 15:32, Alexander Malysh wrote:

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