[reSIProcate] Questions about AresDns::internalInit() method

[Scott Godin]

Hi Dario,

I think you are right.  Calling checkDnsChange is not supposed to modify
the current operation of anything - it is only a check.  I agree we need
the first set of changes you mentioned to the setting of timeout and tries.


Looking closer at the #ifndef USE_CARES block you pointed out - I think
there are bigger problems.  Looks like everytime checkDnsChange is called
the mPollItems array will grow by nServers.  That is not good.  I'm
thinking this block of code needs to move to AresDns::init method instead.
It also needs to be smarter to handle init being called mulitple times.
I'm pretty sure the intention of the developer that added checkDnsChange
was that init should be called again it it returns true - but I'm not 100%
sure.  I am making that assumption for now.

I have checked in a fix for this that Dario has already tested out.

Best Regards,
Scott Godin



On Fri, Jan 2, 2015 at 9:44 AM, Dario Bozzali <Dario.Bozzali at ifmgroup.it>
wrote:

>  Hi all,
>
> I was looking at the file rutil/dns/AresDns.cxx, in particular at
> AresDns::checkDnsChange() and AresDns::internalInit() methods, and I
> noticed some things that I don’t understand.
>
>
>
> Method internalInit() is used in init() and checkDnsChange(), but with
> different parameters. For example, in init() method for timeout and tries
> variables are used the values that have been set using DnsStub::setDnsTimeoutAndTries(),
> but in checkDnsChange() method it is used the default value (0) and in
> internalInit() method mChannel data member is changed instead of (I
> suppose) the local variable *channel (see lines 374-382).
>
>       if (timeout > 0)
>
>       {
>
>          mChannel->timeout = timeout;
>
>       }
>
>
>
>       if (tries > 0)
>
>       {
>
>          mChannel->tries = tries;
>
>       }
>
> I suppose it should be:
>
>       if (timeout > 0)
>
>       {
>
>          (*channel)->timeout = timeout;
>
>       }
>
>
>
>       if (tries > 0)
>
>       {
>
>          (*channel)->tries = tries;
>
>       }
>
>
>
> Moreover I have some doubts about the following lines of code:
>
> #ifndef USE_CARES
>
>       if ( mPollGrp )
>
>       {
>
>          // expand vector to hold {nservers} and init to NULL
>
>          mPollItems.insert( mPollItems.end(), (*channel)->nservers,
> (AresDnsPollItem*)0);
>
>          // tell ares to let us know when things change
>
>          ares_process_set_poll_cb(mChannel,
> AresDnsPollItem::socket_poll_cb, this);
>
>       }
>
> #endif
>
>
>
> In a similar way as exposed before, I don’t undertand why *channel is used
> to change mChannel since if internalInit() is called inside checkDnsChange()
> then *channel is not equal to mChannel as it happens when internalInit() is
> called inside init().
>
> I think that if internalInit() is used by checkDnsChange() the previous
> code could (or should) not be called.
>
>
>
> What is your opinions about my considerations?
>
>
>
> Thank you and kind regards,
>
> Dario.
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20150115/8d8e7276/attachment.htm>