Re: [RFC] gw/smsc/smsc_smpp.c commit to revision 1.85 (amalysh)

Alexander Malysh <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <[email protected]>
Hi Stipe,

Stipe Tolj wrote:
> Hi Alex,
> 
> just reviewing your commit and I have some questions I'd like to hear 
> comments ;)
> 
> * you safe an "empty" function to dump_pdu() via a #define that does a 
> "useless" do loop. Has this been profiled via assembler generation that 
> it is really a benefit against the existing version? Could do some quick 
> tests on this.

it's world known process to tell gcc to not generate any code:
do {} while(0)
and don't break compilation.

as for the benefit, I can tell you: function and macro does the same 
thing and difference is only that one was function that need function 
call, stack building, return path, etc. wat macro doesn't need. Now you 
are free to think what is best for 3 lines of code ;)

> 
> * prototype for handle_dlr() changed in the sense that you pass the 
> Octstr pointer et all to the function and not the SMPP_PDU pointer 
> itself? Why? Makes the function prototype horrible long IMO.

that's pretty simple if you look where this function called. This 
function is shared for data_sm and deliver_sm PDU. And because PDUs are 
different and offset in union too we must have 2 different function to 
handle those. But generic DLR handling shared in this function...

> 
> * everyting else seems straight forward to me ;)
> 

the above points were also straight forward ;)

Thanks,
Alex


> I'm not veto'ing, just want to hear about the "conceptual ideas" you 
> have/had for the above two point.
> 
> Stipe
> 
> mailto:stolj_{at}_wapme-group.de
> -------------------------------------------------------------------
> Wapme Systems AG
> 
> Vogelsanger Weg 80
> 40470 Düsseldorf, NRW, Germany
> 
> phone: +49.211.74845.0
> fax: +49.211.74845.299
> 
> mailto:info_{at}_wapme-systems.de
> http://www.wapme-systems.de/
> -------------------------------------------------------------------
> 
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.