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
