Re: /bzr/squid3/trunk/ r12194: Small optimization in CommOpener statistic accounting.

On 03.07.2012 03:30, Francesco Chemolli wrote:
>
> - if (conn_->getPeer())
> - conn_->getPeer()->stats.conn_open++;
> + if (peer *peer=(conn_->getPeer()))
> + ++peer->stats.conn_open;
>
> lookupLocalAddress();

Two points:

1) assignment in the if() needs to be double-bracketed around the whole
= operator expression, not just the RHS:
     if ((peer *peer=conn_->getPeer()))

2) This is the type of pre-increment operator use which I am a bit
uncomfortable with.

It is hard to tell when skimming the code at speed whether that should
be read as:

   (++peer)->stats.conn_open;
or
   ++(conn_->getPeer()->stats.conn_open);

IMHO we should consistently use bracketing as above to clarify in
situations like this where there is any complex location syntax.

Amos
Received on Mon Jul 02 2012 - 22:34:37 MDT