Re: [PATCH] Get all available TLV's as Meta-Data

My previous patch was wrong, attached one should fix it...

Alexander Malysh schrieb:
> Hi Alex,
> 
> seems your patch has a one issue:
> +                    Octstr *tmptag = octstr_format("%d", opt_tag); \
> +                    struct smpp_tlv *tlv = dict_get(tlv_by_tag, tmptag); \
> +                    if (tlv != NULL) {\
> +                        Octstr *tmpname = octstr_create(#name); \
> +                        Octstr *val = octstr_format("%ld", p->name); \
> +                        dict_put(p->tlv, tmpname, val); \
> 
> you put Spec defined TLV name into dictionary instead of user configured .
> 
> And I found a way to have smaller patch todo it ;)
> How about this one?
> 
> 
> diff --git a/gw/smsc/smpp_pdu.c b/gw/smsc/smpp_pdu.c
> index 2a46e88..cd38f22 100644
> --- a/gw/smsc/smpp_pdu.c
> +++ b/gw/smsc/smpp_pdu.c
> @@ -491,7 +491,8 @@ SMPP_PDU *smpp_pdu_unpack(Octstr *data_without_len)
>                      pos += opt_len; \
>                  } else
>      #define OPTIONAL_END \
> -                { \
> +                {;}\
> +                do { \
>                      Octstr *val = NULL; \
>                      struct smpp_tlv *tlv; \
>                      Octstr *tmp = octstr_format("%ld", opt_tag); \
> @@ -543,7 +544,7 @@ SMPP_PDU *smpp_pdu_unpack(Octstr *data_without_len)
>                          octstr_destroy(val); \
>                          pos += opt_len; \
>                      } \
> -                } \
> +                } while(0); \
>              } \
>          }
>      #define INTEGER(name, octets) \
> 
> Thanks,
> Alex
> 
> Alejandro Guerrieri schrieb:
>> Alex,
>>
>> Done. Please check the attached patch.
>>
>> There's still the issue regarding the "message_id" param on 
>> submit_sm_resp / data_sm_resp.
>>
>> The new match by tag method voids the option of using the same 
>> filtering mechanism mentioned on my previous, mail since it would 
>> require a match by name (or inventing a tag
>> address for message_id, which is not an option IMHO).
>>
>> Since it's not a TLV, does it make sense to add it to meta-data, or 
>> maybe exploring other ways to be able to retrieve it? Maybe a special 
>> % param or a conf directive like "smsc-message-id-into-meta-data" on 
>> the smpp group. Do you have any other ideas about how to achieve this?
>>
>> Regards,
>> -- 
>> Alejandro Guerrieri
>> [email protected]
>>
>>
>>
>> El 26/11/2008, a las 06:43 a.m., Alexander Malysh escribió:
>>
>>> Hi Alex,
>>>
>>> great!!! :)
>>>
>>> Could you please change this patch to use dictionary tlv_by_tag 
>>> because the name of configured TLV may be different of the one in 
>>> SMPP spec. but the tag will be equal. I think it's up to user which 
>>> name configured TLV should use.
>>>
>>> And minor issue: please always make patches from gateway root 
>>> directory with cvs diff -Nau.
>>>
>>> Thanks,
>>> Alex
>>>
>>> Alejandro Guerrieri schrieb:
>>>> Hi,
>>>> This patch allows meta-data to carry all available TLV's not only 
>>>> the User-Defined.
>>>> All TLV's defined on smpp_pdu.def were "hijacked" by the main pdu 
>>>> structure, so they never reached the tlv dictionary. What this patch 
>>>> does is to check for defined TLV's on the smpp-tlv group and copy 
>>>> them to the tlv dictionary. Those TLV's are then available on the 
>>>> meta-data parameter.
>>>> It only copies the TLV's explicitly defined, otherwise the meta-data 
>>>> parameter would be unnecessary cluttered with all available TLV's.
>>>> This solves the "receipted_message_id" issue with deliver_sm (to 
>>>> name one), but does _not_ solve the "message_id" param on 
>>>> submit/deliver/data_sm_response, since message_id is not a TLV.
>>>> For that parameter I could add a call for meta_data_set_value to 
>>>> inject it into the meta data (already tried and works), but then it 
>>>> would be always available.
>>>> To avoid this, I could use the same filtering mechanism as with the 
>>>> TLV's, but that would mean defining a dummy tag address, since this 
>>>> is not a TLV so it doesn't have a documented address. I could filter 
>>>> using tag_by_name, so the address wouldn't matter anyways, but it's 
>>>> somewhat ugly imho.
>>>> Ideas? Opinions?
>>>> Regards,
>>>> -- 
>>>> Alejandro Guerrieri
>>>> [email protected]
>>>
>>>
>>
> 
> 
>

diff --git a/gw/smsc/smpp_pdu.c b/gw/smsc/smpp_pdu.c
index 2a46e88..513150d 100644
--- a/gw/smsc/smpp_pdu.c
+++ b/gw/smsc/smpp_pdu.c
@@ -491,6 +491,7 @@ SMPP_PDU *smpp_pdu_unpack(Octstr *data_without_len)
                     pos += opt_len; \
                 } else
     #define OPTIONAL_END \
+                {;}\
                 { \
                     Octstr *val = NULL; \
                     struct smpp_tlv *tlv; \
@@ -532,17 +533,18 @@ SMPP_PDU *smpp_pdu_unpack(Octstr *data_without_len)
                             panic(0, "SMPP: Internal error, unknown configured TLV type %d.", tlv->type); \
                             break; \
                         } \
-                    } else { \
-                        val = octstr_copy(data_without_len, pos, opt_len); \
-                        if (val) \
-                            octstr_binary_to_hex(val, 0); \
-                        else \
-                            val = octstr_create(""); \
-                        warning(0, "SMPP: Unknown TLV(0x%04lx,0x%04lx,%s) for PDU type (%s) received!", \
-                              opt_tag, opt_len, octstr_get_cstr(val), pdu->type_name); \
-                        octstr_destroy(val); \
-                        pos += opt_len; \
+                        /* go to the next TLV */ \
+                        continue; \
                     } \
+                    val = octstr_copy(data_without_len, pos, opt_len); \
+                    if (val) \
+                        octstr_binary_to_hex(val, 0); \
+                    else \
+                        val = octstr_create(""); \
+                    warning(0, "SMPP: Unknown TLV(0x%04lx,0x%04lx,%s) for PDU type (%s) received!", \
+                          opt_tag, opt_len, octstr_get_cstr(val), pdu->type_name); \
+                    octstr_destroy(val); \
+                    pos += opt_len; \
                 } \
             } \
         }