Re: New Features Question

	  Alex,

> how about somathing like this?
> 
> #define smpp_pdu(name) \
> static int smpp_##name_to_msg(SMPP_PDU pdu, ...)
> {
>     struct name cmd = pdu->u.name;
>     cmd->source_addr = octstr_create(XXX);
>     ...
> }
> smpp_pdu(submit_sm);
> smpp_pdu(data_sm);
> 
> 
> now to patch: please split your patch in changesets (e.g. add data_sm
> handling, add new struct members to msg struct, etc...)
> 
> Thanks in advance!

      Finally got some free time to look back at that code again.  I see 
kind of what you are doing above, but don't really see how it fixes what 
I'm having an issue with.  Could you give a quick example of the 
following code being revamped to work in one equivalent line (so I don't 
have to duplicate code thus making maintenance a nightmare for those who 
come after me)?

---

if ( /* we are doing submit_sm */ )
{
    pdu->u.submit_sm.source_addr = octstr_duplicate(msg->sms.sender);
}
else if ( /* we are doing data_sm */ )
{
    pdu->u.data_sm.source_addr = octstr_duplicate(msg->sms.sender);
}

---

      I mean I understand the structure that is being used above.  It 
seems a bit messy to me as you're allocating the same memory for shared 
fields over and over (ie. source_addr is allocated for both submit_sm 
and data_sm), and yet you have to make sure you fill the correct 
instance for the message you're building.  That is unless the site I saw 
(not a Kannel site) the structure definition style used by the SMPP code 
was lying to me.  :P  I mean, if it were a simple struct like I've used 
for everything I've implemented (GSM SMS, IS-41 SMS, MMS, RADIUS), then 
you'd just write the above code as...

---

pdu.source_addr = octstr_duplicate(msg->sms.sender);

---

      ...and be done with it.  If the message you're sending doesn't use 
source_addr, you may needlessly fill it, but the actual 'encoder' will 
not use it to build the message (as it's smart enough to know not to, 
which it would have to be anyway).  Needlessly allocated memory ensues 
in that case, but the code stays nice and simple.  Perhaps I'm just 
missing something with this allocation style being used?

Jon