Re: [PATCH] Add DNS lookup step to ICAP connect

Oops, forgot to attach the patch.
I am attaching here the v6 version

On 07/12/2011 02:41 AM, Alex Rousskov wrote:
> On 07/01/2011 02:44 PM, Tsantilas Christos wrote:
>
>
>> - theIdleConns("ICAP Service",NULL),
>
> Please initialize to NULL in case we throw before we reach the
> allocation lines below.

Done.

>
>
>> + theIdleConns = new IdleConnList("ICAP Service",NULL);
>> + assert(theIdleConns);
>
> Please remove the assert(). The new operator cannot return NULL.
OK.

>
>
>> + if ((reused = Comm::IsConnOpen(connection))) {
>> debugs(93,3, HERE<< "reused pconn "<< connection);
>> - ++theBusyConns;
>> }
>> + ++theBusyConns;
>>
>> return connection;
>> }
>
> Unless I am missing something, this can be simplified and optimized as:
>
> ++theBusyConns;
> debugs(93,3, HERE<< "got connection: "<< connection);
> return connection;
>
> When the connection is printed, we will see whether it is open, right?

yes, we just need the "reused = Comm::IsConnOpen(connection)" line. Now
exist alone, without the "if" test.

>
> The above changes are cosmetic and should not block the commit if the
> patch passes your tests, of course.

OK.
If there is not any objection I will commit it to trunk.

>
>
> Thank you,
>
> Alex.
>