Hold on, I agree completely with you in terms of performance, but the if statement is not correct the way it stands. Shouldn't it be?
if (!((!trans->ssl && trans-port == 80) || (trans-ssl && trans-port == 443))) octstr_format_append(os, ":%ld", trans->port); BR, Nikos On Sun, Jan 25, 2009 at 6:44 PM, Stipe Tolj <[email protected]> wrote: > Alexander Malysh schrieb: > > Hi, > > > > some comments to patch: > > > > + /* port, only added if literally not default. */ > > + if (trans->port != 80) { > > + octstr_format_append(os, ":%ld", trans->port); > > + } > > > > I don't think this is needed, just always add port to URI. > > yep, but I have speed in mind here. The octstr_format_append() is > "expensiver" > in terms of CPU cycles then a simple long compare in the if statement. And > we > also don't need to "parse" it again in the next integration. That's why. > > > Otherwise patch look ok, +1 from me. > > ok, going to commit. > > Thanks Alejandro and Alex for reviewing. > Stipe > > -- > ------------------------------------------------------------------- > Kölner Landstrasse 419 > 40589 Düsseldorf, NRW, Germany > > tolj.org system architecture Kannel Software Foundation (KSF) > http://www.tolj.org/ http://www.kannel.org/ > > mailto:st_{at}_tolj.org <st_%7Bat%7D_tolj.org> mailto: > stolj_{at}_kannel.org <stolj_%7Bat%7D_kannel.org> > ------------------------------------------------------------------- > >
