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

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