Re: [reSIProcate] AfterSocketCreationFuncPtr not called for TCPsockets

[Scott Godin <sgodin@xxxxxxxxxxxxxxx>]

Good catch.  That solution seems reasonable to me.  The only thing I would change is the name of the new fn - I prefer something like init() instead of ctorHelper(). : )

Scott

On Tue, May 19, 2009 at 9:56 AM, Paul Kurmas <pkurmas@xxxxxxxxxxxxx> wrote:

I would appreciate some feedback from the community about the solution to problem (1) I noted.

The TcpBaseTransport class does a bad thing... it calls InternalTransport::bind() in its ctor.  TcpBaseTransport's ctor is called BEFORE the ctor for TcpTransport or TlsTransport, so we don't have a completely formed object.  Specifically, the virtual function table pointer appropriate for the leaf class is not yet set.  InternalTransport::bind() does not this, calls Transport::transport() -- a pure virtual function.  BOOM.

It seems to me the appropriate solution is to defer the majority of the work done in the TcpBaseTransport ctor until AFTER the ctor for TcpTransport or TlsTransport is completed.  I believe this can be done relatively neatly by introducing a protected member function ctorHelper() to TcpBaseTransport, moving the bulk of the current ctor into this function, then adding a call to ctorHelper() from the TcpTransport and TlsTransport ctors.

Thoughts?

PK

-----Original Message-----
From: Paul Kurmas

Sent: Monday, May 18, 2009 2:21 PM
To: Scott Godin
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] AfterSocketCreationFuncPtr not called for TCPsockets

Thanks for the feedback, Scott.  I'm encountering two issues.
1) In InternalTransport::bind() where the callback function is called, there's a call to the virtual function transport().  For TcpTransport, this is resulting in a run-time failure due to a call to an unimplemented pure virtual function... which is weird because by inspection it *is* defined.  It is inline in the header file, but it ought to be OK.  I confirmed this by replacing the call to transport() with the value 0 and all was well.  Annoying, but probably solvable.
2) I'm successfully modifying the parameters on the server socket, but thanks to a bug/feature in (at least this version of) Linux, those parameters don't carry forth to the sockets created by the call to accept().  So short of modifying TcpBaseTransport::processListen to call the callback function after the call to accept(), there's nothing to be gained by my change.

I'm open to opinions and feedback, though.
Thanks.

PK
________________________________________
From: slgodin@xxxxxxxxx [mailto:slgodin@xxxxxxxxx] On Behalf Of Scott Godin
Sent: Sunday, May 17, 2009 12:12 PM
To: Paul Kurmas
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] AfterSocketCreationFuncPtr not called for TCP sockets

1) Has a similar change been posted as a draft somewhere else?

[Scott] I haven't seen such a change from any other source.

2) Is there a historical reason why the functionality isn't provided for
TCP sockets?

[Scott] I can't think of one.  Sounds like a good addition to the stack.  If you would like to post a patch for this, I can get it committed to SVN. 

Thanks!
Scott
On Fri, May 15, 2009 at 2:52 PM, Paul Kurmas <pkurmas@xxxxxxxxxxxxx> wrote:
One clarification here... the reason for installing the callback
function is to be able to call setsockopt() to set the IP_TTL and IP_TOS
fields of the stream.

Any alternate solutions that use existing APIs to solve the problem
would be welcome.  Thanks again.

PK

-----Original Message-----
From: Paul Kurmas
Sent: Friday, May 15, 2009 2:43 PM
To: 'resiprocate-devel@xxxxxxxxxxxxxxx'
Subject: AfterSocketCreationFuncPtr not

I've created a function complying to the prototype for
AfterSocketCreationFuncPtr & pass it to the SipStack ctor.  However, the
function is not called for TCP sockets.  The argument is dropped from
the call to the TcpTransport ctor, and in turn the TcpBaseTransport
ctor.  The implementation of the TcpBaseTransport ctor list passes a
null pointer to the InternalTransport ctor.

I've verified in the latest-and-greatest code that this hasn't been
changed in v1.4.1.  My codebase is at v1.3.4.  Before I start changing
the implementation & posting the change, I wanted to know two things --
1) Has a similar change been posted as a draft somewhere else?
2) Is there a historical reason why the functionality isn't provided for
TCP sockets?

Many thanks in advance.
PK

_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel