[PATCH] Re: errcode hides Comm::ListenStateData::errcode.

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Tue, 03 Aug 2010 20:33:37 +1200

Attached is a patch which seeks to resolve the errcode clash and produce
the correct documented behaviour and content of both this variable and
the CommAcceptCbParams field related to it.

Please review in light of the comments below.

Alex Rousskov wrote:
> On 08/02/2010 09:12 AM, noc_at_squid-cache.org wrote:
>> "../../../src/comm/ListenStateData.cc", line 189: Error: errcode hides
>> Comm::ListenStateData::errcode.
>
> There are more related problems here, besides name hiding.
>
> * Comm::ListenStateData::setListen() sets errcode member on some
> failures but those failures are not propagated to the caller. That seems
> wrong.

The params.xerrno variable is set from this->errcode and passed to the
caller. However as you notice ignorable errors were ignored completely
and listener goes back to waiting.

In the patch: this->errcode is set if accept() _ever_ errors and updated
with 0 before each re-accept().
  The result is that regardless of the handling logics this->errcode
will contain the errno result from the last accept() attempt.

>
> * The "and accept takes a callback to call" part of the
> Comm::ListenStateData::setListen() comment does not seem to correspond
> to the actual setListen() code. May belong to the constructor instead.

ListenStateData is in need of some serious AsyncJob updates. Now that I
know how to do them properly its on the worklist after cleanup-comm
ConnOpener is completed.

For now documentation updated.

>
> * Comm::ListenStateData::notify sets param.errcode but not this->errcode.
>

Resulting from the inconsistencies in your point #1. Fixed.

> * The errcode member documentation ("errno code if any happened so far")
> is inconsistent with the code because some errors are ignored.
>

Fixed.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.5

Received on Tue Aug 03 2010 - 08:33:43 MDT

This archive was generated by hypermail 2.2.0 : Tue Aug 03 2010 - 12:00:04 MDT