[squid-dev] [PATCH] convert squidaio_ctrl_t to MEMPROXY_CLASS

[Amos Jeffries]

On 2/10/2015 3:29 a.m., Kinkie wrote:
> Hi,
>    the attached large-context patch converts squidaio_ctrl_t to MEMPROXY_CLASS.
> It also:
> - adds a tiny bit of documentation to some fields of squidaio_ctrl_t
> - extends the MEMPROXY_CLASS API to expose the pool's use-count as a
> static function
> - fixes a tiny bit of documentation in aiops.cc
> - fixes a bug in aioWrite [1]
> 
> [1] old code wouldn't set ctrlp->len; when free_func would eventually
> be called, it'd happen with a length argument of 0 (default length
> value). I'm quite surprised this hasn't caused havoc so far. New code
> sets len. So either this fixes an issue, or it introduces one ;)
> 

That sounds like the freeFunc do not use len. Maybe its another bit of
dead code left over from Squid-2 logics?


in src/DiskIO/DiskThreads/DiskThreads.h:
* please reduce the 2 spaces before some "= delete;"
 - also are these default & copy ctor and copy-assign removals actually
needed since there is a ctor with parameters defined?

* "struct squidaio_ctrl_t *next;" is now wrong for a non-struct
 - maybe other uses of "struct squidaio_ctrl_t"


in src/DiskIO/DiskThreads/aiops.cc:

* since the name length of the parameters is causing line wrap, the
answer is to shorten the parameter names.
 - consider abbreviations
 - please use one member per line on the initializer list


in src/DiskIO/DiskThreads/async_io.cc:

* please merge+sort mem/Pool.h into the include list

* please move the "squidaio_ctrl_t *ctrlp;" down to their relevant
position of first use. I can see at least one defined a couple of lines
above the 'ctrlp = new ...' .


Amos